Skip to content

Commit 22c5c2c

Browse files
committed
fix(cli): keep pure-v2 tables on the v2 ladder (#787 review)
The `unresolvedHint` fail-closed added for #772 finding 7 had no `candidates.length > 0` guard, so it fired on pure EQL v2 tables too. `encrypt backfill` records `encryptedColumn` in migrations.json unconditionally, v2 included. On a pure-v2 table `listEncryptedColumns` returns [] (the classifier recognises `eql_v3_*` only), so the hint failed to resolve, `columnExists` found the real `eql_v2_encrypted` column, and `cutover` / `drop` exited 1 — telling users to downgrade for a lifecycle this same build still fully implements in cutover.ts / drop.ts. Gate the fail-closed on a non-empty candidate list. Finding 7's protection is unchanged: the mixed table it targets always has candidates. Order `explainUnresolved`'s empty-candidates fall-through ahead of the hint branch so both agree for direct callers. Also drop the "this release no longer manages that lifecycle" claim from the cutover/drop `via: 'sole'` messages — the build does still implement it; the command simply resolves EQL v3 counterparts only. Tests cover the previously untested shape: candidates [] + a recorded hint. Review follow-ups: - Extend the placeholder-table guard to `encrypt backfill` via loadEncryptionContext; correct the changeset/skill wording to name the commands that actually refuse (cutover/drop never read the client file). - Alias `@cipherstash/migrate` to source in the CLI unit vitest config, so `pnpm --filter stash test` no longer needs a prior workspace build (verified: 888 tests pass with packages/migrate/dist removed). - Revert the unrelated em-dash re-encoding in packages/cli/package.json.
1 parent b609c7d commit 22c5c2c

11 files changed

Lines changed: 237 additions & 40 deletions

File tree

‎.changeset/encrypt-lifecycle-mixed-table.md‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,5 +35,10 @@ all now fixed:
3535
column that actually encrypts the named one, and says explicitly not to record
3636
the guess.
3737

38-
Pure-v2 and pure-v3 tables are unaffected, as are tables with two or more EQL v3
39-
columns (resolution already failed closed there).
38+
The new refusal is scoped to the mixed table it was written for: it applies only
39+
when the table actually holds EQL v3 columns that a guess could wrongly claim. A
40+
pure-v2 table has none, so it still falls through to the EQL v2 lifecycle exactly
41+
as before — including when `encrypt backfill` recorded an `encryptedColumn` for
42+
it, which it does for v2 columns too. Pure-v2 and pure-v3 tables are therefore
43+
unaffected, as are tables with two or more EQL v3 columns (resolution already
44+
failed closed there).

‎.changeset/init-scaffold-compiles.md‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,12 @@ whose looser bound accepted `[]`; collapsing that into an alias of `Encryption`
1313
tightened it.)
1414

1515
The scaffold now declares a single sentinel table, `__stash_placeholder__`, so
16-
the file typechecks as written. `stash encrypt` commands refuse to run while
17-
that table is still the only one declared, and say so — rather than failing
18-
later with a confusing "table not found".
16+
the file typechecks as written. Every command that reads the encryption client
17+
— `stash db push`, `stash db validate`, and `stash encrypt backfill` — refuses
18+
to run while that table is still the only one declared, and names it, rather
19+
than failing later with a confusing "table not found". (`stash encrypt cutover`
20+
and `stash encrypt drop` do not read the client file at all; they resolve
21+
against the database.)
1922

2023
Nothing in the repo compiled this output before: `packages/cli` has no
2124
typecheck step, the codegen tests only string-match fragments of the template,

‎packages/cli/package.json‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
{
22
"name": "stash",
33
"version": "1.0.0-rc.4",
4-
"description": "CipherStash CLI \u2014 the one stash command for auth, init, encryption schema, database setup, and secrets.",
4+
"description": "CipherStash CLI — the one stash command for auth, init, encryption schema, database setup, and secrets.",
55
"repository": {
66
"type": "git",
77
"url": "git+https://github.com/cipherstash/stack.git",
@@ -57,7 +57,7 @@
5757
"posthog-node": "^5.41.0",
5858
"zod": "^3.25.76"
5959
},
60-
"//optionalDependencies": "@cipherstash/auth ships per-platform native bindings as optional peerDependencies. pnpm does not auto-install platform-matched optional peer deps, so we declare them here as optionalDependencies \u2014 pnpm then picks the binary matching the host's os/cpu (from each sub-package's own package.json) and ignores the rest. All seven names share a single catalog entry to keep them in lockstep.",
60+
"//optionalDependencies": "@cipherstash/auth ships per-platform native bindings as optional peerDependencies. pnpm does not auto-install platform-matched optional peer deps, so we declare them here as optionalDependencies — pnpm then picks the binary matching the host's os/cpu (from each sub-package's own package.json) and ignores the rest. All seven names share a single catalog entry to keep them in lockstep.",
6161
"optionalDependencies": {
6262
"@cipherstash/auth-darwin-arm64": "catalog:repo",
6363
"@cipherstash/auth-darwin-x64": "catalog:repo",
Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
import fs from 'node:fs'
2+
import os from 'node:os'
3+
import path from 'node:path'
4+
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
5+
import { PLACEHOLDER_TABLE_NAME } from '@/config/index.js'
6+
7+
/**
8+
* `stash encrypt` does not load its client through `loadEncryptConfig`, so the
9+
* placeholder guard that command group needs lives in `loadEncryptionContext`.
10+
* Both must refuse the un-replaced scaffold; otherwise `requireTable` reports
11+
* `Table "users" was not found … Available: __stash_placeholder__`, which names
12+
* the symptom instead of the cause (#787 review).
13+
*
14+
* Runs against the real jiti runtime — the guard reads the tables harvested
15+
* from an actually-evaluated client module, which is the part worth pinning.
16+
*/
17+
describe('loadEncryptionContext — the un-replaced init scaffold', () => {
18+
let tmpDir: string
19+
let originalCwd: () => string
20+
21+
const writeProject = (clientBody: string) => {
22+
fs.writeFileSync(
23+
path.join(tmpDir, 'stash.config.ts'),
24+
`export default {
25+
databaseUrl: 'postgresql://u:p@127.0.0.1:5432/db',
26+
client: './client.ts',
27+
}`,
28+
)
29+
fs.writeFileSync(path.join(tmpDir, 'client.ts'), clientBody)
30+
process.cwd = () => tmpDir
31+
}
32+
33+
/**
34+
* The duck-typed shapes `loadEncryptionContext` harvests: any export with a
35+
* `getEncryptConfig()` method is the client, and any with `tableName` +
36+
* `build()` is a table. Hand-rolled rather than imported from
37+
* `@cipherstash/stack` so the test needs no native module.
38+
*/
39+
const table = (name: string) =>
40+
`{ tableName: '${name}', build: () => ({ tableName: '${name}', columns: {} }) }`
41+
42+
beforeEach(() => {
43+
tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'stash-encrypt-ctx-'))
44+
originalCwd = process.cwd
45+
})
46+
47+
afterEach(() => {
48+
process.cwd = originalCwd
49+
vi.restoreAllMocks()
50+
if (tmpDir && fs.existsSync(tmpDir)) {
51+
fs.rmSync(tmpDir, { recursive: true, force: true })
52+
}
53+
})
54+
55+
it('exits 1 naming the sentinel when it is the only table declared', async () => {
56+
writeProject(
57+
`export const encryptionClient = { getEncryptConfig: () => ({}) }
58+
export const placeholderTable = ${table(PLACEHOLDER_TABLE_NAME)}`,
59+
)
60+
const error = vi.spyOn(console, 'error').mockImplementation(() => {})
61+
vi.spyOn(process, 'exit').mockImplementation(() => {
62+
throw new Error('process.exit')
63+
})
64+
65+
const { loadEncryptionContext } = await import('../context.js')
66+
await expect(loadEncryptionContext()).rejects.toThrow('process.exit')
67+
68+
const message = error.mock.calls.flat().join('\n')
69+
expect(message).toContain(PLACEHOLDER_TABLE_NAME)
70+
// Names the cause, not `requireTable`'s "table not found" symptom.
71+
expect(message).toContain('still contains the placeholder table')
72+
expect(message).not.toContain('was not found in the encryption client')
73+
})
74+
75+
it('allows the sentinel through once a real table sits alongside it', async () => {
76+
// Only the SOLE-placeholder case is the un-replaced scaffold. A user who
77+
// has added real tables must not be blocked by a leftover sentinel export.
78+
writeProject(
79+
`export const encryptionClient = { getEncryptConfig: () => ({}) }
80+
export const placeholderTable = ${table(PLACEHOLDER_TABLE_NAME)}
81+
export const users = ${table('users')}`,
82+
)
83+
84+
const { loadEncryptionContext } = await import('../context.js')
85+
const ctx = await loadEncryptionContext()
86+
87+
expect(ctx.tables.has('users')).toBe(true)
88+
})
89+
})

‎packages/cli/src/commands/encrypt/context.ts‎

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,11 @@
11
import fs from 'node:fs'
22
import path from 'node:path'
33
import type { EncryptionClient } from '@cipherstash/stack/encryption'
4-
import { loadStashConfig, type ResolvedStashConfig } from '@/config/index.js'
4+
import {
5+
loadStashConfig,
6+
PLACEHOLDER_TABLE_NAME,
7+
type ResolvedStashConfig,
8+
} from '@/config/index.js'
59

610
/**
711
* Structural shape of `@cipherstash/stack`'s `EncryptedTable` class.
@@ -138,6 +142,20 @@ export async function loadEncryptionContext(): Promise<EncryptionContext> {
138142
process.exit(1)
139143
}
140144

145+
// Same guard `loadEncryptConfig` applies for `stash db push` / `db validate`,
146+
// repeated here because `stash encrypt` does not go through that loader. The
147+
// scaffold `stash init` writes declares one sentinel table so the file
148+
// compiles; reaching here with only that table means it was never replaced.
149+
// Without this, `requireTable` reported `Table "users" was not found …
150+
// Available: __stash_placeholder__`, which names the symptom and not the
151+
// cause (#787 review).
152+
if (tables.size === 1 && tables.has(PLACEHOLDER_TABLE_NAME)) {
153+
console.error(
154+
`Error: ${stashConfig.client} still contains the placeholder table \`${PLACEHOLDER_TABLE_NAME}\` that \`stash init\` wrote.\n\nDeclare your encrypted columns and pass those tables to Encryption({ schemas: [...] }) in that file, then re-run this command.`,
155+
)
156+
process.exit(1)
157+
}
158+
141159
return { stashConfig, client, tables }
142160
}
143161

‎packages/cli/src/commands/encrypt/cutover.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ export async function cutoverCommand(options: CutoverCommandOptions) {
107107
// the same reason (#772 review, finding 7).
108108
if (info.via === 'sole') {
109109
p.log.error(
110-
`${options.table}.${encryptedColumn} (${info.domain}) is the table's only EQL v3 column, but nothing confirms it encrypts "${options.column}" — refusing to report a cut-over outcome on that guess. If "${options.column}" pairs with a legacy eql_v2_encrypted column, this release no longer manages that lifecycle. Otherwise record the pairing: re-run \`stash encrypt backfill --table ${options.table} --column ${options.column} --encrypted-column <the column that actually encrypts ${options.column}>\`.`,
110+
`${options.table}.${encryptedColumn} (${info.domain}) is the table's only EQL v3 column, but nothing confirms it encrypts "${options.column}" — refusing to report a cut-over outcome on that guess. If "${options.column}" pairs with a legacy eql_v2_encrypted column, resolution cannot see it (this command resolves EQL v3 counterparts only) — drive that column's v2 lifecycle against its own encrypted column directly. Otherwise record the pairing: re-run \`stash encrypt backfill --table ${options.table} --column ${options.column} --encrypted-column <the column that actually encrypts ${options.column}>\`.`,
111111
)
112112
exitCode = 1
113113
return

‎packages/cli/src/commands/encrypt/drop.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,7 @@ export async function dropCommand(options: DropCommandOptions) {
113113
// live `DROP COLUMN` on the plaintext at exit 0 (#772 review, finding 7).
114114
if (info?.via === 'sole') {
115115
p.log.error(
116-
`${options.table}.${info.column} (${info.domain}) is the table's only encrypted column, but nothing confirms it encrypts "${options.column}" — refusing to generate an irreversible drop on that guess. Identify the column that actually encrypts "${options.column}" and record that pairing: re-run \`stash encrypt backfill --table ${options.table} --column ${options.column} --encrypted-column <name>\` (which writes it to the manifest), or set "encryptedColumn" for this column in .cipherstash/migrations.json. If "${options.column}" pairs with a legacy eql_v2_encrypted column, this release no longer manages that lifecycle — do not record ${info.column}.`,
116+
`${options.table}.${info.column} (${info.domain}) is the table's only encrypted column, but nothing confirms it encrypts "${options.column}" — refusing to generate an irreversible drop on that guess. Identify the column that actually encrypts "${options.column}" and record that pairing: re-run \`stash encrypt backfill --table ${options.table} --column ${options.column} --encrypted-column <name>\` (which writes it to the manifest), or set "encryptedColumn" for this column in .cipherstash/migrations.json. If "${options.column}" pairs with a legacy eql_v2_encrypted column, resolution cannot see it (this command resolves EQL v3 counterparts only) — drive that column's v2 lifecycle against its own encrypted column directly, and do not record ${info.column}.`,
117117
)
118118
exitCode = 1
119119
return

‎packages/cli/src/commands/encrypt/lib/__tests__/resolve-eql.test.ts‎

Lines changed: 50 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,17 @@
44
*
55
* `listEncryptedColumns` can no longer emit `version: 2` — a legacy
66
* `eql_v2_encrypted` column is not classified as an EQL column at all, so it
7-
* never reaches this function as a candidate. The post-cutover v2 state (the
8-
* ciphertext renamed onto the plaintext column's own name) therefore arrives
9-
* here as an EMPTY candidate list, which the first guard already falls through
10-
* on. These tests exist so removing the now-unreachable `version === 2` branch
11-
* is provably behaviour-preserving, and so a future v2 sweep cannot delete the
12-
* empty-list guard the v2 lifecycle actually depends on.
7+
* never reaches this function as a candidate. Every pure-v2 table therefore
8+
* arrives here as an EMPTY candidate list, both pre-cutover (`<col>` /
9+
* `<col>_encrypted`) and post-cutover (the ciphertext renamed onto the
10+
* plaintext column's own name), and the first guard falls through on it.
11+
*
12+
* These tests exist so removing the now-unreachable `version === 2` branch is
13+
* provably behaviour-preserving, and so a future v2 sweep cannot delete the
14+
* empty-list guard the v2 lifecycle actually depends on. That guard has to hold
15+
* even when the manifest recorded an `encryptedColumn` — `backfill` records one
16+
* for v2 columns too — which is the `candidates.length > 0` gate on the
17+
* fail-closed path (#787 review).
1318
*/
1419

1520
import type { EncryptedColumnInfo } from '@cipherstash/migrate'
@@ -64,6 +69,15 @@ describe('explainUnresolved', () => {
6469
expect(explainUnresolved('users', 'email', [])).toBeNull()
6570
})
6671

72+
it('still falls through when no EQL v3 columns exist BUT a hint was recorded', () => {
73+
// The pure-v2 table. `encrypt backfill` records `encryptedColumn` for v2
74+
// columns too, so a hint is present on every table backfilled with this
75+
// release — it must not flip the empty-candidate fall-through into a
76+
// refusal, because `cutover` / `drop` in this same build still implement
77+
// the v2 ladder this falls through to (#787 review).
78+
expect(explainUnresolved('users', 'ssn', [], 'ssn_encrypted')).toBeNull()
79+
})
80+
6781
it('fails closed, naming every candidate, when none is identifiable', () => {
6882
const message = explainUnresolved('users', 'email', [
6983
v3('a_enc'),
@@ -133,6 +147,36 @@ describe('resolveColumnLifecycle — a recorded hint that is not a v3 candidate'
133147
expect(unresolvedHint).toBe('ssn_encrypted')
134148
})
135149

150+
// The pure-v2 shape, and the regression the `candidates.length > 0` gate
151+
// exists to prevent (#787 review). `backfill` records `encryptedColumn`
152+
// unconditionally — v2 included — so EVERY pure-v2 table backfilled with this
153+
// release carries a hint naming a real, existing, non-v3 column. Without the
154+
// gate, `columnExists` returned true, `unresolvedHint` was set, and
155+
// `cutover` / `drop` refused a lifecycle this same build still performs.
156+
it('does not fail closed on a pure-v2 table, where no v3 column can be mis-claimed', async () => {
157+
// No v3 columns at all: just the `ssn` / `ssn_encrypted` v2 pair, which the
158+
// classifier does not see.
159+
listEncryptedColumns.mockResolvedValue([])
160+
readManifest.mockResolvedValue({
161+
tables: { users: [{ column: 'ssn', encryptedColumn: 'ssn_encrypted' }] },
162+
})
163+
164+
const { info, candidates, unresolvedHint } = await resolveColumnLifecycle(
165+
clientWithColumns('ssn', 'ssn_encrypted'),
166+
'users',
167+
'ssn',
168+
)
169+
170+
expect(info).toBeNull()
171+
expect(candidates).toEqual([])
172+
// The fall-through signal: no hint reported, so `explainUnresolved` returns
173+
// null and the caller reaches its own v2 preconditions.
174+
expect(unresolvedHint).toBeUndefined()
175+
expect(
176+
explainUnresolved('users', 'ssn', candidates, unresolvedHint),
177+
).toBeNull()
178+
})
179+
136180
it('explains the recorded counterpart by name rather than listing candidates', async () => {
137181
const message = explainUnresolved(
138182
'users',

0 commit comments

Comments
 (0)