Skip to content

Commit e326757

Browse files
committed
fix(cli,scripts): close the review nits, and the same defect where it repeats
Remediation for the two review rounds on #789, plus what verifying them turned up in the same code. **The linter fix the review asked for twice.** `livePackages` moving from `readdirSync` to `git ls-files` was the highest-value change in the PR and had no test that pinned it. It still doesn't fail under the revert anyone would actually write: keeping the git call and unioning the filesystem back in passes every existing test while fully restoring the false negative. The new fixture builds the discriminating case — a package deleted from git whose gitignored `dist/` and `node_modules/` shells survive on disk — and is the only test that fails under both a naive `readdirSync` revert and that hybrid. The `git status` guard is scoped to the probe, not to `packages/`, so an unrelated uncommitted file doesn't turn it into an assertion about nothing. **A missing target no longer passes in silence.** A linter whose entire job is catching dead paths in configuration skipped dead paths in its own: rename a target and it dropped out of coverage forever, green. Its sibling already exits 2 for a stale allowlist entry. Both now do, and both report a target outside the repo by name instead of as a `../../../../..` chain climbing out of the root — the review's cosmetic nit, which turned out to sit in both files. **Every step that names an integration-specific API now branches.** Step 5 was fixed for this; steps 1, 2 and the read-path step were not. A Prisma Next project was sent at `types.*` / `encryptedTable` — the client `stash schema build` explicitly refuses to scaffold for it — and a plain-Postgres project was pointed three times at "the integration skill" it never gets installed. `encryptQuery` is shown taking the schema objects rather than an object-shorthand that read as three required strings. `queryOperatorGuidance` is a switch with a neutral default rather than an if-chain ending in Drizzle's answer: tsup transpiles without type-checking and this package has no typecheck script, so nothing would have caught a fifth integration inheriting it. One test asserted nothing: it scoped to a `#### Encryption cutover` heading that does not exist, and `substring(-1)` returns the whole document.
1 parent c081ea4 commit e326757

8 files changed

Lines changed: 338 additions & 31 deletions

File tree

.changeset/decrypt-chaining-docs.md

Lines changed: 24 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,27 @@ with no lock-context argument.
2323

2424
Also fixes the setup prompt `stash init` writes for coding agents, which
2525
referenced `protectOps.eq` — an API that does not exist anywhere in the repo.
26-
The step now names the query API each integration can actually import:
27-
`createEncryptionOperators(client)` (conventionally `ops`) for Drizzle, the
28-
`encryptedSupabase` wrapper's own filters for Supabase, the `eql*` column
29-
operators for Prisma Next, and `client.encryptQuery(...)` for a plain Postgres
30-
project — which is also pointed at `stash-encryption`, since it is installed
31-
with no integration skill.
26+
Every step naming an integration-specific API now branches on the project's
27+
actual integration, instead of naming Drizzle's and Supabase's and leaving the
28+
other two to guess:
29+
30+
- **Query paths.** `createEncryptionOperators(client)` (conventionally `ops`)
31+
for Drizzle, the `encryptedSupabase` wrapper's own filters for Supabase, the
32+
`eql*` column operators for Prisma Next, and `client.encryptQuery(...)` for a
33+
plain Postgres project.
34+
- **Schema authoring.** The `types.*` column factories for Drizzle, the
35+
`eql_v3_encrypted` domain in migration SQL for Supabase, the `cipherstash.*`
36+
field constructors in `schema.prisma` for Prisma Next, and `encryptedTable`
37+
for plain Postgres. Prisma Next was previously sent at `types.*` /
38+
`encryptedTable` — the client `stash schema build` explicitly refuses to
39+
scaffold for that integration.
40+
- **Read paths.** `decryptModel(row, usersSchema)` where that applies, and the
41+
wrapper's transparent decryption where it does not.
42+
- **Skill pointers.** A plain Postgres project installs no integration-specific
43+
skill, so each "see the integration skill" was a pointer at a file that was
44+
never written. Those now point at `stash-encryption`, which it does get.
45+
46+
`client.encryptQuery` is also shown taking the schema objects themselves
47+
(`{ table: usersSchema, column: usersSchema.email }`) rather than an
48+
object-shorthand that read as three required strings — `queryType` is inferred
49+
from the column's configured indexes.

packages/cli/src/commands/init/lib/__tests__/setup-prompt.test.ts

Lines changed: 78 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,78 @@ describe('renderSetupPrompt — orient + route (implement mode)', () => {
142142
expect(out).toContain('encryptQuery')
143143
expect(out).toContain('stash-encryption')
144144
})
145+
146+
// `EncryptQueryOptions` takes the schema OBJECTS — `usersSchema` and
147+
// `usersSchema.email` — not their names as strings, and `queryType` is
148+
// inferred from the column's indexes when omitted. The object-shorthand
149+
// `{ table, column, queryType }` reads as three required string fields:
150+
// the same species of plausible-but-wrong API as the `protectOps.eq` this
151+
// PR exists to have removed.
152+
it('shows encryptQuery taking schema objects, not string names', () => {
153+
const out = render('postgresql')
154+
expect(out).toContain('column: usersSchema.email')
155+
expect(out).not.toContain('{ table, column, queryType }')
156+
})
157+
158+
// `packages/cli` builds with tsup, which transpiles without type-checking,
159+
// and has no `typecheck` script — so `Record<Integration, …>` buys nothing
160+
// at build time here. An if-chain ending in a bare Drizzle `return` hands a
161+
// future fifth integration the exact string this helper exists to stop it
162+
// getting. Degrade to neutral guidance, as `skillsFor()` degrades to
163+
// BASE_SKILLS.
164+
it('degrades to neutral guidance for an unrecognised integration', () => {
165+
const out = renderSetupPrompt({
166+
...baseCtx,
167+
integration: 'mystery-orm' as SetupPromptContext['integration'],
168+
})
169+
expect(out).not.toContain('createEncryptionOperators')
170+
expect(out).not.toContain('encryptedSupabase')
171+
expect(out).toContain('encryptQuery')
172+
})
173+
})
174+
175+
// Step 2 named the Drizzle and Supabase schema APIs unconditionally — the
176+
// identical defect step 5 above was just fixed for. A prisma-next project was
177+
// sent at `types.*` / `encryptedTable`, which `stash schema build` explicitly
178+
// refuses to scaffold for that integration; a plain-Postgres project was
179+
// given no named path at all.
180+
describe('schema-authoring guidance is per-integration', () => {
181+
const render = (integration: SetupPromptContext['integration']) =>
182+
renderSetupPrompt({ ...baseCtx, integration })
183+
184+
it('names the Drizzle column factories for drizzle', () => {
185+
const out = render('drizzle')
186+
expect(out).toContain('`types.*` column factories')
187+
expect(out).toContain('@cipherstash/stack-drizzle')
188+
})
189+
190+
it('names the eql_v3 domain for supabase', () => {
191+
expect(render('supabase')).toContain('eql_v3_encrypted')
192+
})
193+
194+
it('names the cipherstash.* field constructors for prisma-next', () => {
195+
const out = render('prisma-next')
196+
expect(out).toContain('cipherstash.TextSearch()')
197+
expect(out).toContain('prisma/schema.prisma')
198+
// The `types.*` client is precisely what `stash schema build` refuses to
199+
// emit for prisma-next.
200+
expect(out).not.toContain('`types.*` column factories')
201+
})
202+
203+
it('names encryptedTable for plain postgresql', () => {
204+
const out = render('postgresql')
205+
expect(out).toContain('encryptedTable')
206+
expect(out).toContain('@cipherstash/stack/v3')
207+
})
208+
})
209+
210+
// `postgresql` installs no integration-specific skill (SKILL_MAP), so every
211+
// unconditional "see the integration skill" is a pointer at a file that was
212+
// never written. Step 5's was fixed; two more survived elsewhere.
213+
it('never points plain postgresql at "the integration skill"', () => {
214+
const out = renderSetupPrompt({ ...baseCtx, integration: 'postgresql' })
215+
expect(out).not.toMatch(/the integration skill/i)
216+
expect(out).toContain('stash-encryption')
145217
})
146218

147219
it('emits supabase migration commands for supabase integration', () => {
@@ -477,7 +549,12 @@ describe('renderSetupPrompt — no db push recommendations', () => {
477549
expect(out).toMatch(/1\.\s*\*\*Schema-add/)
478550
expect(out).toMatch(/2\.\s*\*\*Dual-write/)
479551
// Cutover is still covered, just without a db push workaround note.
480-
const cutoverSection = out.substring(out.indexOf('#### Encryption cutover'))
552+
// The heading has to be one that exists: `indexOf` returning -1 makes
553+
// `substring(-1)` the whole document, so this scoped assertion was
554+
// silently asserting nothing at all.
555+
const heading = '#### Backfill and switch'
556+
expect(out).toContain(heading)
557+
const cutoverSection = out.substring(out.indexOf(heading))
481558
expect(cutoverSection).toMatch(/encrypt cutover/)
482559
})
483560

packages/cli/src/commands/init/lib/setup-prompt.ts

Lines changed: 78 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -68,28 +68,92 @@ function migrationCommands(
6868
return undefined
6969
}
7070

71+
/**
72+
* Where this integration's rules were actually written.
73+
*
74+
* `postgresql` is the fallback integration and installs no integration-specific
75+
* skill (see `SKILL_MAP` in `install-skills.ts`), so "the integration skill" is
76+
* a pointer at a file that was never created. Point it at `stash-encryption`,
77+
* which it does get.
78+
*/
79+
function integrationSkillRef(integration: Integration): string {
80+
switch (integration) {
81+
case 'drizzle':
82+
case 'supabase':
83+
case 'prisma-next':
84+
return 'the integration skill'
85+
default:
86+
return 'the `stash-encryption` skill'
87+
}
88+
}
89+
90+
/**
91+
* How this integration declares an encrypted column in the schema.
92+
*
93+
* Per-integration for the same reason the query operators are: the APIs are not
94+
* interchangeable, and `stash schema build` (`utils.ts`) refuses outright to
95+
* scaffold a `types.*` client for prisma-next — that integration authors its
96+
* columns with `cipherstash.*` constructors in `schema.prisma` instead.
97+
*/
98+
function schemaAuthoringGuidance(integration: Integration): string {
99+
switch (integration) {
100+
case 'drizzle':
101+
return 'Declare it with the `types.*` column factories from `@cipherstash/stack-drizzle` on the existing `pgTable`'
102+
case 'supabase':
103+
return 'Declare the column with the `eql_v3_encrypted` domain in the migration SQL (`encryptedSupabase` derives its encryption config by introspecting those domains)'
104+
case 'prisma-next':
105+
return 'Declare the field with the `cipherstash.*` constructors in `prisma/schema.prisma` (`cipherstash.TextSearch()`, `cipherstash.DoubleOrd()`, …), with the `cipherstash` extension pack wired up per `@cipherstash/prisma-next/control`'
106+
default:
107+
return 'Declare the table with `encryptedTable` and the `types.*` domain factories from `@cipherstash/stack/v3`, then pass it to `Encryption({ schemas })`'
108+
}
109+
}
110+
71111
/**
72112
* How this integration filters on an encrypted column. Named per-integration
73113
* rather than generically because the APIs are not interchangeable and only one
74114
* of them is importable from any given project: `createEncryptionOperators` is
75115
* exported by `@cipherstash/stack-drizzle` alone, so naming it for a Supabase
76116
* or Prisma project sends the agent after a package that is not installed.
77117
*
78-
* `postgresql` is the fallback integration and is installed with no integration
79-
* skill (see `SKILL_MAP` in `install-skills.ts`), so it is pointed at
80-
* `stash-encryption` instead of "the integration skill".
118+
* A `switch` with a neutral `default`, not an if-chain ending in the Drizzle
119+
* string: `packages/cli` is built by tsup, which transpiles without
120+
* type-checking, and the package has no `typecheck` script — so nothing would
121+
* catch a fifth `Integration` variant silently inheriting Drizzle's answer.
122+
* `skillsFor()` in `install-skills.ts` degrades the same way, for the same
123+
* reason.
81124
*/
82125
function queryOperatorGuidance(integration: Integration): string {
83-
if (integration === 'supabase') {
84-
return 'query paths filter through the `encryptedSupabase` wrapper (`es.from("users").select(...).eq("email", value)`) — it encrypts filter operands for encrypted columns automatically; see the integration skill'
126+
switch (integration) {
127+
case 'supabase':
128+
return 'query paths filter through the `encryptedSupabase` wrapper (`es.from("users").select(...).eq("email", value)`) — it encrypts filter operands for encrypted columns automatically; see the integration skill'
129+
case 'prisma-next':
130+
return 'query paths use the `eql*` operators on the column inside `.where()` (`u.email.eqlEq(value)`, `eqlMatch`, `eqlGt`, …) — see the integration skill'
131+
case 'drizzle':
132+
return 'query paths use the right operator (`ops.eq`, from `createEncryptionOperators(client)`) — see the integration skill'
133+
default:
134+
// `table` and `column` are the schema OBJECTS, not their names as
135+
// strings, and `queryType` is inferred from the column's configured
136+
// indexes unless it is passed to override the inference.
137+
return 'query paths encrypt the search term first — `client.encryptQuery(value, { table: usersSchema, column: usersSchema.email })`, passing the schema objects themselves rather than their names — and compare that against the encrypted column; see the `stash-encryption` skill (a plain Postgres project gets no integration-specific skill)'
85138
}
86-
if (integration === 'prisma-next') {
87-
return 'query paths use the `eql*` operators on the column inside `.where()` (`u.email.eqlEq(value)`, `eqlMatch`, `eqlGt`, …) — see the integration skill'
88-
}
89-
if (integration === 'postgresql') {
90-
return 'query paths encrypt the search term first with `client.encryptQuery(value, { table, column, queryType })` and compare against the encrypted column — see the `stash-encryption` skill (a plain Postgres project gets no integration skill)'
139+
}
140+
141+
/**
142+
* How this integration turns ciphertext back into values on the read path.
143+
*
144+
* Named per-integration for the third time in this file, and for the third
145+
* time because the answer is not portable: `decryptModel` is the typed
146+
* client's, transparent decryption is the Supabase wrapper's.
147+
*/
148+
function readPathGuidance(integration: Integration): string {
149+
switch (integration) {
150+
case 'supabase':
151+
return 'selects through the `encryptedSupabase` wrapper decrypt transparently'
152+
case 'prisma-next':
153+
return 'the encrypted fields decrypt through the Prisma Next client'
154+
default:
155+
return 'call `decryptModel(row, usersSchema)` — or `bulkDecryptModels` for a set — before returning the value to callers'
91156
}
92-
return 'query paths use the right operator (`ops.eq`, from `createEncryptionOperators(client)`) — see the integration skill'
93157
}
94158

95159
function bullet(line: string): string {
@@ -298,10 +362,10 @@ export function renderImplementPrompt(ctx: SetupPromptContext): string {
298362
'',
299363
'### Add a new encrypted column',
300364
'',
301-
'Use when the column **does not yet exist** in the database (no plaintext predecessor to preserve). This is normal Drizzle / Supabase work plus the encryption client patterns from the integration skill.',
365+
`Use when the column **does not yet exist** in the database (no plaintext predecessor to preserve). This is normal schema work in the project's own ORM or migration tooling, plus the encryption client patterns from ${integrationSkillRef(ctx.integration)}.`,
302366
'',
303367
"1. **If this is the first encrypted column in the project, configure the bundler exclusion first.** `@cipherstash/stack` cannot be bundled (it wraps a native FFI module). Next.js: add `serverExternalPackages: ['@cipherstash/stack', '@cipherstash/protect-ffi']` to `next.config.*`. Webpack: `externals`. esbuild: `external`. Vite SSR: `ssr.external`. Without this, the encryption client crashes at runtime with `Cannot find module '@cipherstash/protect-ffi-*'`. See the `stash-encryption` skill's Installation section for the full snippets.",
304-
"2. Edit the user's real schema file (`src/db/schema.ts` or wherever they keep it) to declare the new encrypted column. Use the patterns in the integration skill — the `types.*` domain factories from `@cipherstash/stack-drizzle` for Drizzle, and the `types.*` factories from `@cipherstash/stack/eql/v3` (via `encryptedTable`, passed as `schemas`) for Supabase. Encrypted columns must be **nullable `jsonb`** at creation time. Never `.notNull()`.",
368+
`2. Edit the user's real schema file (\`src/db/schema.ts\` or wherever they keep it) to declare the new encrypted column. ${schemaAuthoringGuidance(ctx.integration)} — the patterns are in ${integrationSkillRef(ctx.integration)}. Encrypted columns must be **nullable \`jsonb\`** at creation time (the \`eql_v3_*\` domains are over \`jsonb\`). Never \`.notNull()\`.`,
305369
`3. Generate the schema migration${migration ? ` — \`${migration.generate}\` (${migration.tool})` : " using the project's existing migration tooling"}.`,
306370
`4. Show the user the generated SQL before applying${migration ? ` — \`${migration.apply}\`` : ''}.`,
307371
`5. Wire the column through the application code: insert paths encrypt before write, select paths decrypt after read, ${queryOperatorGuidance(ctx.integration)}.`,
@@ -328,7 +392,7 @@ export function renderImplementPrompt(ctx: SetupPromptContext): string {
328392
'',
329393
`3. **Backfill.** Run \`${cli} encrypt backfill --table <T> --column <c>\`. The CLI prompts the user (or accepts \`--confirm-dual-writes-deployed\` non-interactively) to confirm dual-writes are live, then chunks through the existing rows. Resumable; checkpoints to \`cs_migrations\` after every chunk. SIGINT-safe.`,
330394
`4. **Switch reads to the encrypted column.** The step depends on the EQL version (\`${cli} encrypt backfill\` prints it; \`${cli} encrypt status\` shows it). **EQL v3 (the default):** there is no rename — update the schema and queries to read/write the encrypted column by its own name, and wire decryption through the encryption client. **EQL v2 (legacy data only):** update the schema file to declare the encrypted column under its final name (drop the twin suffix), then \`${cli} encrypt cutover --table <T> --column <c>\` runs the rename in one transaction (\`<col>\` → \`<col>_plaintext\`, twin → \`<col>\`). Do **not** declare a v2 column with a \`types.*\` domain — those are EQL v3 only. The adapters no longer author v2 (\`@cipherstash/stack-drizzle\` removed \`encryptedType\`), so a v2 column is a read path: declare it with the deprecated \`@cipherstash/stack/schema\` builders and decrypt through \`@cipherstash/stack\`.`,
331-
'5. **Wire the read path through the encryption client.** The read column now holds ciphertext. Read code paths must decrypt before returning the value to callers — `decryptModel(row, table)` for Drizzle, the `encryptedSupabase` wrapper for Supabase, or the equivalent `decrypt`/`bulkDecryptModels` calls. Without this step, your read paths return raw encrypted payloads to end users. The integration skill has the exact API.',
395+
`5. **Wire the read path through the encryption client.** The read column now holds ciphertext${readPathGuidance(ctx.integration)}. Without this step, your read paths return raw encrypted payloads to end users. See ${integrationSkillRef(ctx.integration)} for the exact API.`,
332396
'6. **Remove the dual-write code.** The plaintext column (still `<col>` on v3; renamed `<col>_plaintext` on v2) is no longer authoritative. Delete the dual-write logic from the persistence layer.',
333397
`7. **Drop.** Run \`${cli} encrypt drop --table <T> --column <c>\`. Generates a migration that removes the now-unused plaintext column (on v3 it first verifies no rows are still plaintext-only). Apply with the project's normal migration tooling.`,
334398
'',
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
# A deleted package whose build output survives on disk
2+
3+
The old thing lived in packages/lint-dead-shell-probe, deleted from git but
4+
still sitting there as a `dist/` shell on any checkout that once built it.

0 commit comments

Comments
 (0)