Skip to content

Commit a7814ea

Browse files
tobyhedetobyhede
committed
fix(stack): bulk-model id split corrupts field names containing hyphens
The bulk model helpers key per-field operation results by `${modelIndex}-${fieldKey}` ids and reconstructed models with a naive key.split('-'), truncating any field key containing a hyphen (some-field -> some): the value landed under the truncated key and the real field silently vanished. Duplicated across all four multi-model reconstruction sites. Extract one fieldsForModelIndex helper that splits at the FIRST hyphen only and use it at all four sites. Offline regression tests (mocked protect-ffi) prove hyphenated and multi-hyphen field names survive bulkEncryptModels / bulkDecryptModels -- both fail against the naive split. Re-expressed from james/cip-3291-bigint-stack 632bdf6.
1 parent 63ca540 commit a7814ea

2 files changed

Lines changed: 143 additions & 44 deletions

File tree

‎packages/stack/__tests__/bulk-model-hyphen-fields.test.ts‎

Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,118 @@
1+
/**
2+
* Regression tests for bulk model reconstruction with hyphenated field names.
3+
*
4+
* The bulk model helpers key per-field operation results by a
5+
* `${modelIndex}-${fieldKey}` id. Reconstruction previously split that id
6+
* with a naive `split('-')`, truncating any field key containing a hyphen
7+
* (`some-field` → `some`) — the encrypted/decrypted value then landed under
8+
* the truncated key and the real field silently vanished from the model.
9+
* The split now happens at the FIRST hyphen only (`fieldsForModelIndex` in
10+
* src/encryption/helpers/model-helpers.ts).
11+
*/
12+
13+
import { beforeEach, describe, expect, it, vi } from 'vitest'
14+
import type { EncryptionClient } from '@/encryption'
15+
import { Encryption } from '@/index'
16+
import { encryptedColumn, encryptedTable } from '@/schema'
17+
18+
// A protect-ffi-shaped encrypted payload carrying its plaintext in `c` so
19+
// the fake decrypt can undo it (and `isEncryptedPayload` detects it).
20+
const enc = (plaintext: unknown) => ({
21+
v: 2,
22+
i: { t: 'users', c: 'col' },
23+
c: `ct:${String(plaintext)}`,
24+
})
25+
26+
vi.mock('@cipherstash/protect-ffi', () => ({
27+
newClient: vi.fn(async () => ({ __mock: 'client' })),
28+
encrypt: vi.fn(async () => enc('x')),
29+
decrypt: vi.fn(async () => 'decrypted'),
30+
encryptBulk: vi.fn(
31+
async (_c: unknown, opts: { plaintexts: Array<{ plaintext: unknown }> }) =>
32+
opts.plaintexts.map((p) => enc(p.plaintext)),
33+
),
34+
decryptBulk: vi.fn(
35+
async (
36+
_c: unknown,
37+
opts: { ciphertexts: Array<{ ciphertext: { c: string } }> },
38+
) => opts.ciphertexts.map((e) => e.ciphertext.c.replace(/^ct:/, '')),
39+
),
40+
decryptBulkFallible: vi.fn(
41+
async (
42+
_c: unknown,
43+
opts: { ciphertexts: Array<{ ciphertext: { c: string } }> },
44+
) =>
45+
opts.ciphertexts.map((e) => ({
46+
data: e.ciphertext.c.replace(/^ct:/, ''),
47+
})),
48+
),
49+
encryptQuery: vi.fn(async () => enc('x')),
50+
encryptQueryBulk: vi.fn(async (_c: unknown, opts: { queries: unknown[] }) =>
51+
opts.queries.map(() => enc('x')),
52+
),
53+
}))
54+
55+
// DB column names deliberately contain hyphens — legal in (quoted) Postgres
56+
// identifiers and previously corrupted by the naive id split.
57+
const users = encryptedTable('users', {
58+
'some-field': encryptedColumn('some-field').equality(),
59+
'multi-part-name': encryptedColumn('multi-part-name').equality(),
60+
plainName: encryptedColumn('plainName').equality(),
61+
})
62+
63+
let client: EncryptionClient
64+
65+
beforeEach(async () => {
66+
vi.clearAllMocks()
67+
client = await Encryption({ schemas: [users] })
68+
})
69+
70+
// biome-ignore lint/suspicious/noExplicitAny: test helper unwraps Result
71+
function unwrap(result: any) {
72+
if (result.failure) {
73+
throw new Error(`operation failed: ${result.failure.message}`)
74+
}
75+
return result.data
76+
}
77+
78+
describe('bulk model helpers with hyphenated field names', () => {
79+
it('bulkEncryptModels keeps hyphenated field names intact', async () => {
80+
const models = [
81+
{ 'some-field': 'a', 'multi-part-name': 'b', plainName: 'c', other: 1 },
82+
{ 'some-field': 'd', plainName: 'e', other: 2 },
83+
]
84+
85+
const encrypted = unwrap(await client.bulkEncryptModels(models, users))
86+
87+
expect(encrypted).toHaveLength(2)
88+
// The full hyphenated keys survive — no truncated `some` / `multi` keys.
89+
expect(encrypted[0]['some-field']).toMatchObject({ c: 'ct:a' })
90+
expect(encrypted[0]['multi-part-name']).toMatchObject({ c: 'ct:b' })
91+
expect(encrypted[0].plainName).toMatchObject({ c: 'ct:c' })
92+
expect(encrypted[0]).not.toHaveProperty('some')
93+
expect(encrypted[0]).not.toHaveProperty('multi')
94+
expect(encrypted[0].other).toBe(1)
95+
expect(encrypted[1]['some-field']).toMatchObject({ c: 'ct:d' })
96+
})
97+
98+
it('bulkDecryptModels keeps hyphenated field names intact', async () => {
99+
const models = [
100+
{
101+
'some-field': enc('a'),
102+
'multi-part-name': enc('b'),
103+
other: 1,
104+
},
105+
{ 'some-field': enc('c'), other: 2 },
106+
]
107+
108+
const decrypted = unwrap(await client.bulkDecryptModels(models))
109+
110+
expect(decrypted).toHaveLength(2)
111+
expect(decrypted[0]['some-field']).toBe('a')
112+
expect(decrypted[0]['multi-part-name']).toBe('b')
113+
expect(decrypted[0]).not.toHaveProperty('some')
114+
expect(decrypted[0]).not.toHaveProperty('multi')
115+
expect(decrypted[0].other).toBe(1)
116+
expect(decrypted[1]['some-field']).toBe('c')
117+
})
118+
})

‎packages/stack/src/encryption/helpers/model-helpers.ts‎

Lines changed: 25 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -644,6 +644,27 @@ function prepareBulkModelsForOperation<T extends Record<string, unknown>>(
644644
return { otherFields, operationFields, keyMap, nullFields }
645645
}
646646

647+
/**
648+
* Collect the per-model fields out of a bulk-operation result map keyed by
649+
* `${modelIndex}-${fieldKey}` ids, splitting each id at the FIRST hyphen
650+
* only. Field keys may themselves contain hyphens (a `some-field` column, or
651+
* a nested `profile.some-field` path), so a naive `split('-')` would truncate
652+
* the field key at its first hyphen and silently drop the value during model
653+
* reconstruction.
654+
*/
655+
function fieldsForModelIndex(
656+
fields: Record<string, unknown>,
657+
modelIndex: number,
658+
): Record<string, unknown> {
659+
const result: Record<string, unknown> = {}
660+
for (const [id, value] of Object.entries(fields)) {
661+
const sep = id.indexOf('-')
662+
if (Number.parseInt(id.slice(0, sep), 10) !== modelIndex) continue
663+
result[id.slice(sep + 1)] = value
664+
}
665+
return result
666+
}
667+
647668
/**
648669
* Helper function to convert multiple decrypted models to models with encrypted fields
649670
*/
@@ -694,17 +715,7 @@ export async function bulkEncryptModels(
694715
}
695716

696717
// Then, reconstruct the encrypted fields
697-
const modelData = Object.fromEntries(
698-
Object.entries(encryptedData)
699-
.filter(([key]) => {
700-
const [idx] = key.split('-')
701-
return Number.parseInt(idx) === modelIndex
702-
})
703-
.map(([key, value]) => {
704-
const [_, fieldKey] = key.split('-')
705-
return [fieldKey, value]
706-
}),
707-
)
718+
const modelData = fieldsForModelIndex(encryptedData, modelIndex)
708719

709720
for (const [key, value] of Object.entries(modelData)) {
710721
const parts = key.split('.')
@@ -761,17 +772,7 @@ export async function bulkDecryptModels<T extends Record<string, unknown>>(
761772
}
762773

763774
// Then, reconstruct the decrypted fields
764-
const modelData = Object.fromEntries(
765-
Object.entries(decryptedFields)
766-
.filter(([key]) => {
767-
const [idx] = key.split('-')
768-
return Number.parseInt(idx) === modelIndex
769-
})
770-
.map(([key, value]) => {
771-
const [_, fieldKey] = key.split('-')
772-
return [fieldKey, value]
773-
}),
774-
)
775+
const modelData = fieldsForModelIndex(decryptedFields, modelIndex)
775776

776777
for (const [key, value] of Object.entries(modelData)) {
777778
const parts = key.split('.')
@@ -833,17 +834,7 @@ export async function bulkDecryptModelsWithLockContext<
833834
}
834835

835836
// Then, reconstruct the decrypted fields
836-
const modelData = Object.fromEntries(
837-
Object.entries(decryptedFields)
838-
.filter(([key]) => {
839-
const [idx] = key.split('-')
840-
return Number.parseInt(idx) === modelIndex
841-
})
842-
.map(([key, value]) => {
843-
const [_, fieldKey] = key.split('-')
844-
return [fieldKey, value]
845-
}),
846-
)
837+
const modelData = fieldsForModelIndex(decryptedFields, modelIndex)
847838

848839
for (const [key, value] of Object.entries(modelData)) {
849840
const parts = key.split('.')
@@ -907,17 +898,7 @@ export async function bulkEncryptModelsWithLockContext(
907898
}
908899

909900
// Then, reconstruct the encrypted fields
910-
const modelData = Object.fromEntries(
911-
Object.entries(encryptedData)
912-
.filter(([key]) => {
913-
const [idx] = key.split('-')
914-
return Number.parseInt(idx) === modelIndex
915-
})
916-
.map(([key, value]) => {
917-
const [_, fieldKey] = key.split('-')
918-
return [fieldKey, value]
919-
}),
920-
)
901+
const modelData = fieldsForModelIndex(encryptedData, modelIndex)
921902

922903
for (const [key, value] of Object.entries(modelData)) {
923904
const parts = key.split('.')

0 commit comments

Comments
 (0)