fix(cli): harden stash eql migration per review (#691)

Addresses the review on #691 — correctness, security, telemetry, and coverage:

- Run the PROJECT-LOCAL drizzle-kit (`pnpm exec` / `npx --no-install`), not the
  `pnpm dlx`/`yarn dlx` download-and-run form, so it resolves the project's own
  drizzle.config.ts + schema. New shared `execCommand`/`execArgv` in init/utils
  (setup-prompt's private copy folded in).
- Invoke via `spawnSync` with an argv array instead of `execSync` on a shell
  string — a `--name` with spaces or shell metacharacters is now one inert token
  (no word-splitting, no command injection). Plus a `[\w-]+` name guard.
- Pass `--out` through to `drizzle-kit generate` (always) so the flag actually
  steers where the migration is written, and our lookup can't miss it.
- Validation exits and every abort throw `CliExit(1)` instead of `process.exit`,
  so the telemetry `finally` runs and the branches are unit-testable.
- Guard `loadBundledEqlSql` (corrupt bundle → clean error, not a fatal trace),
  add the missing `p.intro`, and call `printNextSteps()` so the now-preferred
  install path isn't less helpful than `eql install`.
- HELP banner lists `eql migration`.

Tests: 14 migration unit (target selection incl. `--supabase`-alone, name guard,
dry-run, argv threading, non-zero drizzle-kit, write-failure cleanup, SQL order),
3 `findGeneratedMigration` unit (now public API), e2e smoke + `eql migration
--help`. 557 unit / 65 e2e green, code:check clean.

Claude-Session: https://claude.ai/code/session_01MxTTPaPP16m6br7Hoab94w

Author: coderdan

packages/cli/src/bin/main.ts

@@ -108,6 +108,7 @@ Commands:
   telemetry <sub>      Manage anonymous usage analytics (status, enable, disable)
 
   eql install          Scaffold stash.config.ts (if missing) and install EQL extensions
+  eql migration        Generate an EQL v3 install migration for your ORM (Drizzle)
   eql upgrade          Upgrade EQL extensions to the latest version
   eql status           Show EQL installation status
 

packages/cli/src/cli/registry.ts

@@ -405,13 +405,13 @@ export const registry: CommandGroup[] = [
             name: '--name',
             value: '<name>',
             description:
-              'Name for the generated migration (Drizzle). Defaults to `install-eql`.',
+              'Name for the generated migration (Drizzle). Letters, numbers, dashes, underscores only. Defaults to `install-eql`.',
           },
           {
             name: '--out',
             value: '<path>',
             description:
-              'Directory to write the generated migration into (Drizzle). Defaults to `drizzle`.',
+              'Directory drizzle-kit writes the migration into (passed to `drizzle-kit generate --out`). Defaults to `drizzle`; set it to match your drizzle.config.ts.',
           },
           DRY_RUN_FLAG,
         ],

packages/cli/src/commands/db/__tests__/find-generated-migration.test.ts

@@ -0,0 +1,49 @@
+import { mkdtempSync, rmSync, writeFileSync } from 'node:fs'
+import { tmpdir } from 'node:os'
+import { join } from 'node:path'
+import { afterEach, beforeEach, describe, expect, it } from 'vitest'
+import { findGeneratedMigration } from '../install.js'
+
+/**
+ * `findGeneratedMigration` was promoted to public API by the `eql migration`
+ * command and now has two consumers (`db install --drizzle` and
+ * `eql migration --drizzle`), so its branches are pinned directly — a change for
+ * one consumer must not silently break the other.
+ */
+describe('findGeneratedMigration', () => {
+  let dir: string
+  beforeEach(() => {
+    dir = mkdtempSync(join(tmpdir(), 'stash-find-migration-'))
+  })
+  afterEach(() => {
+    rmSync(dir, { recursive: true, force: true })
+  })
+
+  it('throws when the out directory does not exist', async () => {
+    await expect(
+      findGeneratedMigration(join(dir, 'nope'), 'install-eql'),
+    ).rejects.toThrow(/output directory not found/)
+  })
+
+  it('throws when no .sql file matches the migration name', async () => {
+    writeFileSync(join(dir, '0000_other.sql'), '')
+    await expect(findGeneratedMigration(dir, 'install-eql')).rejects.toThrow(
+      /Could not find a migration matching "install-eql"/,
+    )
+  })
+
+  it('returns the highest-numbered match, ignoring non-.sql and non-matching entries', async () => {
+    for (const f of [
+      '0000_install-eql.sql',
+      '0010_install-eql.sql',
+      '0011_install-eql.txt', // not .sql
+      '0001_users.sql', // doesn't match the name
+    ]) {
+      writeFileSync(join(dir, f), '')
+    }
+    // Relies on drizzle-kit's zero-padded 4-digit prefix for lexical == numeric.
+    expect(await findGeneratedMigration(dir, 'install-eql')).toBe(
+      join(dir, '0010_install-eql.sql'),
+    )
+  })
+})

packages/cli/src/commands/db/install.ts

@@ -400,7 +400,7 @@ function resolveProviderOptions(
   return { supabase, drizzle, excludeOperatorFamily }
 }
 
-function printNextSteps(): void {
+export function printNextSteps(): void {
   p.note(
     [
       'Your project is set up. To encrypt your first column, pick the path',

packages/cli/src/commands/eql/__tests__/migration.test.ts

@@ -1,20 +1,79 @@
-import { describe, expect, it } from 'vitest'
-import { buildEqlV3MigrationSql } from '../migration.js'
+import {
+  existsSync,
+  mkdirSync,
+  mkdtempSync,
+  readFileSync,
+  rmSync,
+  writeFileSync,
+} from 'node:fs'
+import { tmpdir } from 'node:os'
+import { join } from 'node:path'
+import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
+import { CliExit } from '../../../cli/exit.js'
+import { messages } from '../../../messages.js'
+import { buildEqlV3MigrationSql, eqlMigrationCommand } from '../migration.js'
+
+// clack is chrome — silence it and spy on the error/note channels the command
+// reports through.
+const clack = vi.hoisted(() => ({
+  spinnerInstance: { start: vi.fn(), stop: vi.fn() },
+  log: { info: vi.fn(), warn: vi.fn(), error: vi.fn(), success: vi.fn() },
+  intro: vi.fn(),
+  note: vi.fn(),
+  outro: vi.fn(),
+}))
+vi.mock('@clack/prompts', () => ({
+  spinner: vi.fn(() => clack.spinnerInstance),
+  log: clack.log,
+  intro: clack.intro,
+  note: clack.note,
+  outro: clack.outro,
+}))
+
+// Stub the drizzle-kit scaffold — only the child process is faked.
+const spawnMock = vi.hoisted(() => vi.fn())
+vi.mock('node:child_process', () => ({ spawnSync: spawnMock }))
+
+// `node:fs` stays REAL by default (so `loadBundledEqlSql` reads the bundled SQL
+// and the tmpdir writes/reads work) — only `writeFileSync` is a spy that
+// delegates to the real impl, so the cleanup test can make just the SQL write
+// throw without touching everything else. `beforeEach` restores the delegating
+// default after `clearAllMocks`.
+const fsWrite = vi.hoisted(() => ({
+  real: undefined as unknown as typeof import('node:fs').writeFileSync,
+  spy: vi.fn(),
+}))
+vi.mock('node:fs', async (importOriginal) => {
+  const actual = await importOriginal<typeof import('node:fs')>()
+  fsWrite.real = actual.writeFileSync
+  return { ...actual, default: actual, writeFileSync: fsWrite.spy }
+})
+
+// `printNextSteps` lives in the install module, which drags in `pg`. Stub it;
+// the two helpers we reuse (`findGeneratedMigration`, `cleanupMigrationFile`)
+// stay real and act on the tmpdir.
+vi.mock('../../db/install.js', async (importOriginal) => {
+  const actual = await importOriginal<typeof import('../../db/install.js')>()
+  return { ...actual, printNextSteps: vi.fn() }
+})
+
+beforeEach(() => {
+  fsWrite.spy.mockImplementation(fsWrite.real)
+})
+afterEach(() => {
+  vi.clearAllMocks()
+})
 
 /**
- * `buildEqlV3MigrationSql` is the pure core of `stash eql migration --drizzle`:
- * it assembles the migration contents from the CLI's bundled v3 install SQL,
- * the optional Supabase grants, and the `cs_migrations` tracking schema. The
- * file-writing orchestration around it (drizzle-kit scaffold + inject) is thin
- * and I/O-bound, so the assembly is where the contract lives.
+ * `buildEqlV3MigrationSql` is the pure core: it assembles the migration from the
+ * CLI's bundled v3 install SQL, the optional Supabase grants, and the
+ * `cs_migrations` tracking schema.
  */
 describe('buildEqlV3MigrationSql', () => {
   it('emits the EQL v3 install bundle and the cs_migrations tracking schema', () => {
     const sql = buildEqlV3MigrationSql({ supabase: false })
-    // v3 bundle, not v2 — the whole point of the command.
     expect(sql).toContain('EQL v3 schema creation')
     expect(sql).toContain('eql_v3')
-    // Bundled tracking schema so one migration run is enough for `stash encrypt`.
     expect(sql).toContain('cs_migrations')
   })
 
@@ -35,12 +94,145 @@ describe('buildEqlV3MigrationSql', () => {
     )
   })
 
-  it('is a superset of the non-supabase content when --supabase is set', () => {
-    const base = buildEqlV3MigrationSql({ supabase: false })
-    const supa = buildEqlV3MigrationSql({ supabase: true })
-    // The grants are additive: everything in the base still appears.
-    expect(supa.length).toBeGreaterThan(base.length)
-    expect(supa).toContain('EQL v3 schema creation')
-    expect(supa).toContain('cs_migrations')
+  it('orders the bundle: schema creation → grants → tracking schema', () => {
+    // Order is the contract: `GRANT ... ON SCHEMA eql_v3` against a not-yet-
+    // created schema is a hard error at migrate time, so `toContain` isn't
+    // enough — the offsets must be monotonic.
+    const sql = buildEqlV3MigrationSql({ supabase: true })
+    const schemaAt = sql.indexOf('EQL v3 schema creation')
+    const grantAt = sql.indexOf('GRANT USAGE ON SCHEMA eql_v3 TO')
+    const trackingAt = sql.indexOf(
+      '-- CipherStash encryption-migration tracking schema.',
+    )
+    expect(schemaAt).toBeGreaterThanOrEqual(0)
+    expect(grantAt).toBeGreaterThan(schemaAt)
+    expect(trackingAt).toBeGreaterThan(grantAt)
+  })
+})
+
+describe('eqlMigrationCommand — target selection', () => {
+  it.each([
+    ['no target', {}, () => messages.eql.migrationNeedsTarget],
+    [
+      'both targets',
+      { drizzle: true, prisma: true },
+      () => messages.eql.migrationOneTarget,
+    ],
+    [
+      '--prisma',
+      { prisma: true },
+      () => messages.eql.migrationPrismaUnavailable,
+    ],
+    // `--supabase` is a modifier, not a target.
+    [
+      '--supabase alone',
+      { supabase: true },
+      () => messages.eql.migrationNeedsTarget,
+    ],
+  ])('exits 1 with an actionable message for %s', async (_label, opts, msg) => {
+    await expect(eqlMigrationCommand(opts)).rejects.toBeInstanceOf(CliExit)
+    expect(clack.log.error).toHaveBeenCalledWith(msg())
+    expect(spawnMock).not.toHaveBeenCalled()
+  })
+})
+
+describe('eqlMigrationCommand — Drizzle', () => {
+  let tmp: string
+  beforeEach(() => {
+    tmp = mkdtempSync(join(tmpdir(), 'stash-eql-migration-'))
+  })
+  afterEach(() => {
+    rmSync(tmp, { recursive: true, force: true })
+  })
+
+  it('rejects a migration name with unsafe characters before spawning', async () => {
+    await expect(
+      eqlMigrationCommand({ drizzle: true, name: 'a b; rm -rf /' }),
+    ).rejects.toBeInstanceOf(CliExit)
+    expect(clack.log.error).toHaveBeenCalledWith(messages.eql.migrationBadName)
+    expect(spawnMock).not.toHaveBeenCalled()
+  })
+
+  it('dry run neither spawns drizzle-kit nor creates the out directory', async () => {
+    const out = join(tmp, 'drizzle')
+    await eqlMigrationCommand({ drizzle: true, dryRun: true, out })
+    expect(spawnMock).not.toHaveBeenCalled()
+    expect(existsSync(out)).toBe(false)
+    expect(clack.note).toHaveBeenCalledWith(
+      expect.stringContaining(
+        'drizzle-kit generate --custom --name=install-eql',
+      ),
+      'Dry Run',
+    )
+  })
+
+  it('dry run says the grants would be included under --supabase', async () => {
+    await eqlMigrationCommand({
+      drizzle: true,
+      dryRun: true,
+      supabase: true,
+      out: join(tmp, 'd'),
+    })
+    expect(clack.note).toHaveBeenCalledWith(
+      expect.stringContaining('(with Supabase grants)'),
+      'Dry Run',
+    )
+  })
+
+  it('threads --name/--out into drizzle-kit (as argv, no shell) and writes the SQL', async () => {
+    const out = join(tmp, 'db', 'migrations')
+    mkdirSync(out, { recursive: true })
+    // Stand in for drizzle-kit scaffolding an empty custom migration.
+    spawnMock.mockImplementation(() => {
+      writeFileSync(join(out, '0000_add-eql.sql'), '')
+      return { status: 0, stdout: '', stderr: '' }
+    })
+
+    await eqlMigrationCommand({ drizzle: true, name: 'add-eql', out })
+
+    // argv array, never a shell string — the name/out are discrete tokens.
+    const [, argv] = spawnMock.mock.calls[0]
+    expect(argv).toContain('drizzle-kit')
+    expect(argv).toContain('--name=add-eql')
+    expect(argv).toContain(`--out=${out}`)
+
+    const written = readFileSync(join(out, '0000_add-eql.sql'), 'utf-8')
+    expect(written).toContain('EQL v3 schema creation')
+    expect(written).toContain('cs_migrations')
+  })
+
+  it('aborts (exit 1) when drizzle-kit exits non-zero', async () => {
+    spawnMock.mockReturnValue({ status: 1, stdout: '', stderr: 'boom' })
+    await expect(
+      eqlMigrationCommand({ drizzle: true, out: join(tmp, 'drizzle') }),
+    ).rejects.toBeInstanceOf(CliExit)
+    expect(clack.log.error).toHaveBeenCalledWith('boom')
+  })
+
+  it('removes the scaffolded migration when writing the SQL fails', async () => {
+    const out = join(tmp, 'drizzle')
+    mkdirSync(out, { recursive: true })
+    const scaffolded = join(out, '0000_install-eql.sql')
+    spawnMock.mockImplementation(() => {
+      // drizzle-kit's empty custom migration (delegates to real write).
+      fsWrite.real(scaffolded, '')
+      return { status: 0, stdout: '', stderr: '' }
+    })
+    // Throw on the SQL write specifically (the big bundle), letting the empty
+    // scaffold write through — robust to call order.
+    fsWrite.spy.mockImplementation(((path: string, data: unknown, ...rest) => {
+      if (typeof data === 'string' && data.includes('EQL v3 schema creation')) {
+        throw new Error('EACCES: permission denied')
+      }
+      return fsWrite.real(path, data as string, ...(rest as []))
+    }) as typeof import('node:fs').writeFileSync)
+
+    await expect(
+      eqlMigrationCommand({ drizzle: true, out }),
+    ).rejects.toBeInstanceOf(CliExit)
+
+    // The empty scaffold must not survive — drizzle-kit would happily run it.
+    expect(existsSync(scaffolded)).toBe(false)
+    expect(clack.log.error).toHaveBeenCalledWith('EACCES: permission denied')
   })
 })