diff --git a/.changeset/cli-load-stash-config-unwrap.md b/.changeset/cli-load-stash-config-unwrap.md new file mode 100644 index 00000000..376a2649 --- /dev/null +++ b/.changeset/cli-load-stash-config-unwrap.md @@ -0,0 +1,17 @@ +--- +'@cipherstash/cli': minor +--- + +Fix `loadStashConfig` to correctly unwrap the default export from `stash.config.ts`. Previously, any database-touching command (`db install`, `db push`, `db validate`, `db status`, `db test-connection`, `schema build`) would fail validation against a perfectly valid config with: + +``` +Error: Invalid stash.config.ts + + - databaseUrl: Invalid input: expected nonoptional, received undefined +``` + +The issue: in jiti 2.x, the `interopDefault: true` option passed to `createJiti(...)` only applies to the deprecated synchronous `jiti(id)` callable form. The async `jiti.import()` ignores it and always returns the full module namespace. With `export default defineConfig({...})` that meant Zod was validating `{ default: { databaseUrl, client } }` and reporting `databaseUrl` as undefined even when the user's config plainly set it. + +Switched to jiti's per-call `{ default: true }` option, which does work on `jiti.import()`. Added an integration test that exercises real jiti against a real temp `stash.config.ts` so future regressions get caught — the previous mocked test was passing the bug straight through. + +This bug surfaced after `db install` started loading `stash.config.ts` (during the onboarding overhaul), but affected every other command that reads the config. diff --git a/packages/cli/src/__tests__/config-jiti-integration.test.ts b/packages/cli/src/__tests__/config-jiti-integration.test.ts new file mode 100644 index 00000000..c6bad3c2 --- /dev/null +++ b/packages/cli/src/__tests__/config-jiti-integration.test.ts @@ -0,0 +1,95 @@ +import fs from 'node:fs' +import os from 'node:os' +import path from 'node:path' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +/** + * Integration test for `loadStashConfig` against the *real* jiti runtime. + * + * The companion file `config.test.ts` mocks the `jiti` module entirely, + * which is fast but can't catch wrapper/unwrap regressions in how the + * default export is returned. This file deliberately does NOT mock jiti — + * it writes a real `stash.config.ts` into a temp dir and asserts that + * `loadStashConfig` returns the inner config rather than the module + * namespace. Regression net for #374: in jiti 2.x the constructor's + * `interopDefault: true` does not apply to `.import()`, so the per-call + * `{ default: true }` option is required. + */ + +describe('loadStashConfig — real jiti', () => { + let tmpDir: string + let originalCwd: () => string + let originalEnv: string | undefined + + beforeEach(() => { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'stash-config-real-jiti-')) + originalCwd = process.cwd + originalEnv = process.env.STASH_TEST_DATABASE_URL + }) + + afterEach(() => { + process.cwd = originalCwd + if (originalEnv === undefined) { + // biome-ignore lint/performance/noDelete: process.env.X = undefined coerces to the string "undefined" in Node, not an unset. + delete process.env.STASH_TEST_DATABASE_URL + } else { + process.env.STASH_TEST_DATABASE_URL = originalEnv + } + + if (tmpDir && fs.existsSync(tmpDir)) { + fs.rmSync(tmpDir, { recursive: true, force: true }) + } + }) + + it('unwraps `export default {...}` to the inner config (#374 regression)', async () => { + // Test-namespaced env var, not `DATABASE_URL`, to avoid clobbering a + // value a developer may have set in their shell or `.env`. The bug is + // about jiti's default-export wrapping, not env-var resolution — the + // `process.env.X` reference inside the config is just an arbitrary + // expression demonstrating that the file body actually evaluated. + process.env.STASH_TEST_DATABASE_URL = + 'postgresql://postgres:postgres@127.0.0.1:54322/postgres' + fs.writeFileSync( + path.join(tmpDir, 'stash.config.ts'), + `export default { + databaseUrl: process.env.STASH_TEST_DATABASE_URL, + client: './src/encryption/index.ts', + }`, + ) + process.cwd = () => tmpDir + + const { loadStashConfig } = await import('@/config/index.ts') + const config = await loadStashConfig() + + expect(config).toEqual({ + databaseUrl: 'postgresql://postgres:postgres@127.0.0.1:54322/postgres', + client: './src/encryption/index.ts', + }) + }) + + it('reports a useful error when databaseUrl is genuinely missing', async () => { + // biome-ignore lint/performance/noDelete: see afterEach above; need an actual unset. + delete process.env.STASH_TEST_DATABASE_URL + fs.writeFileSync( + path.join(tmpDir, 'stash.config.ts'), + `export default { + databaseUrl: process.env.STASH_TEST_DATABASE_URL, + }`, + ) + process.cwd = () => tmpDir + + const errSpy = vi + .spyOn(console, 'error') + .mockImplementation(() => undefined) + vi.spyOn(process, 'exit').mockImplementation(() => { + throw new Error('process.exit') + }) + + const { loadStashConfig } = await import('@/config/index.ts') + await expect(loadStashConfig()).rejects.toThrow('process.exit') + + const allCalls = errSpy.mock.calls.flat().join('\n') + expect(allCalls).toContain('Invalid stash.config.ts') + expect(allCalls).toContain('databaseUrl') + }) +}) diff --git a/packages/cli/src/config/index.ts b/packages/cli/src/config/index.ts index f532b280..8c35a6b6 100644 --- a/packages/cli/src/config/index.ts +++ b/packages/cli/src/config/index.ts @@ -97,13 +97,19 @@ Create a ${CONFIG_FILENAME} file in your project root: } const { createJiti } = await import('jiti') - const jiti = createJiti(configPath, { - interopDefault: true, - }) + const jiti = createJiti(configPath) let rawConfig: unknown try { - rawConfig = await jiti.import(configPath) + // The per-call `{ default: true }` option is the jiti 2.x way to ask + // for the default export to be unwrapped. The `interopDefault` + // *constructor* option only applies to the deprecated synchronous + // `jiti(id)` callable form — `jiti.import()` silently ignores it and + // returns the full module namespace (`{ default: { ... } }`). That + // wrapper would then fail Zod validation with a misleading + // "databaseUrl: received undefined" even when the user's config sets + // it (#374). + rawConfig = await jiti.import(configPath, { default: true }) } catch (error) { console.error(`Error: Failed to load ${CONFIG_FILENAME} at ${configPath}\n`) console.error(error) @@ -148,12 +154,13 @@ export async function loadEncryptConfig( } const { createJiti } = await import('jiti') - const jiti = createJiti(resolvedPath, { - interopDefault: true, - }) + const jiti = createJiti(resolvedPath) let moduleExports: Record try { + // No `{ default: true }` here — we want the full module namespace so + // `Object.values` can find an EncryptionClient regardless of whether + // the user re-exports it as `default` or as a named binding. moduleExports = (await jiti.import(resolvedPath)) as Record } catch (error) { console.error(