diff --git a/packages/sim-setup/src/checks.test.ts b/packages/sim-setup/src/checks.test.ts index 6c19ca4e53c..9157d66bbb8 100644 --- a/packages/sim-setup/src/checks.test.ts +++ b/packages/sim-setup/src/checks.test.ts @@ -1,6 +1,21 @@ -import { describe, expect, it } from 'vitest' -import { type CheckContext, runChecks } from './checks' -import type { EnvFile, EnvTarget } from './env-files' +import { mkdirSync, readFileSync, rmSync, writeFileSync } from 'node:fs' +import path from 'node:path' +import { afterAll, beforeEach, describe, expect, it, vi } from 'vitest' + +const { fakeRoot } = vi.hoisted(() => { + const { mkdtempSync } = require('node:fs') as typeof import('node:fs') + const { tmpdir } = require('node:os') as typeof import('node:os') + const { join } = require('node:path') as typeof import('node:path') + return { fakeRoot: mkdtempSync(join(tmpdir(), 'sim-setup-checks-')) } +}) + +vi.mock('./context', async (importOriginal) => { + const actual = await importOriginal() + return { ...actual, SETUP_CONTEXT: { kind: 'source', root: fakeRoot } } +}) + +import { type CheckContext, loadCheckContext, runChecks } from './checks' +import { type EnvFile, type EnvTarget, parseEnv } from './env-files' function envFile(target: EnvTarget, values: Record = {}): EnvFile { return { @@ -9,6 +24,7 @@ function envFile(target: EnvTarget, values: Record = {}): EnvFil exists: target === 'root', content: '', vars: new Map(Object.entries(values)), + verbatim: new Map(Object.entries(values)), } } @@ -65,3 +81,83 @@ describe('setup coherence checks', () => { }) }) }) + +describe('doctor --fix on env files', () => { + const files = { + sim: path.join(fakeRoot, 'apps/sim/.env'), + realtime: path.join(fakeRoot, 'apps/realtime/.env'), + db: path.join(fakeRoot, 'packages/db/.env'), + } + const referencingDsn = `postgresql://\${DB_USER}@localhost/sim` + + function writeFiles(contents: Record) { + for (const [target, content] of Object.entries(contents)) { + const file = files[target as keyof typeof files] + mkdirSync(path.dirname(file), { recursive: true }) + writeFileSync(file, content) + } + } + + async function fixConsistency() { + const findings = await runChecks(loadCheckContext(false), ['consistency']) + for (const finding of findings) finding.autofix?.() + return findings.find((finding) => finding.message.startsWith('DATABASE_URL differs')) + } + + beforeEach(() => rmSync(fakeRoot, { recursive: true, force: true })) + afterAll(() => rmSync(fakeRoot, { recursive: true, force: true })) + + it.each([ + ['a different URL', 'DATABASE_URL=postgresql://other@localhost/sim\n'], + [ + 'the reference escaped into a literal', + `DATABASE_URL="postgresql://\\\${DB_USER}@localhost/sim"\n`, + ], + ])('reports a $VAR DATABASE_URL against %s and leaves it for a hand copy', async (_, dbFile) => { + writeFiles({ + sim: `DB_USER=alice\nDATABASE_URL=${referencingDsn}\n`, + realtime: '', + db: dbFile, + }) + + const finding = await fixConsistency() + + expect(finding).toMatchObject({ status: 'fail', autofix: undefined }) + expect(readFileSync(files.db, 'utf8')).toBe(dbFile) + }) + + it.each([ + ['missing', 'DATABASE_URL=postgresql://localhost/sim\n'], + ['a placeholder', 'BETTER_AUTH_SECRET=your_auth_secret_here\n'], + ])( + 'never generates a shared secret it cannot copy when realtime has it %s', + async (_, realtime) => { + const secret = `BETTER_AUTH_SECRET=\${AUTH_SECRET_BASE}0123456789abcdef0123456789abcdef` + writeFiles({ sim: `${secret}\n`, realtime, db: '' }) + + for (const finding of await runChecks(loadCheckContext(false), ['schema'])) + finding.autofix?.() + + const after = parseEnv(readFileSync(files.realtime, 'utf8')).get('BETTER_AUTH_SECRET') + expect(after).toBe(parseEnv(realtime).get('BETTER_AUTH_SECRET')) + } + ) + + it('reports a value it cannot write instead of aborting the other fixes', async () => { + const unwritable = 'DATABASE_URL="postgresql://u:p\\$x\\y@localhost/sim"\n' + const dbFile = 'DATABASE_URL=postgresql://other@localhost/sim\n' + writeFiles({ sim: unwritable, realtime: '', db: dbFile }) + + const finding = await fixConsistency() + + expect(finding).toMatchObject({ status: 'fail', autofix: undefined }) + expect(readFileSync(files.db, 'utf8')).toBe(dbFile) + }) + + it('accepts a $VAR DATABASE_URL written the same way in both files', async () => { + const line = `DATABASE_URL=${referencingDsn}\n` + writeFiles({ sim: `DB_USER=alice\n${line}`, realtime: '', db: line }) + + expect(await fixConsistency()).toBeUndefined() + }) +}) diff --git a/packages/sim-setup/src/checks.ts b/packages/sim-setup/src/checks.ts index a129fee3bc1..b6474b1a961 100644 --- a/packages/sim-setup/src/checks.ts +++ b/packages/sim-setup/src/checks.ts @@ -17,6 +17,7 @@ import { portOpen } from './detect' import { type EnvFile, type EnvTarget, + envValueRefusal, generateSecret, isPlaceholder, isTruthy, @@ -30,6 +31,35 @@ import { import { httpHealth, pgProbe, redisPing } from './probes' import { FLAG_TWINS, LOGIN_PROVIDERS } from './twins' +/** + * Whether `key` is written with an unescaped `$`, a reference Next, Bun, and + * Compose expand against the file it sits in. Copied to another file it can + * resolve differently, and copied as a literal it stops expanding, so doctor + * leaves such values for the user to mirror by hand. + */ +function holdsReference(file: EnvFile, key: string): boolean { + return /(^|[^\\])\$/.test(file.verbatim.get(key) ?? '') +} + +/** + * Whether `key` holds the same thing in both files. Parsed values hide the one + * difference that matters for `$`: `${VAR}` expands while `"\${VAR}"` does not. + */ +function sameEnvValue(a: EnvFile, b: EnvFile, key: string): boolean { + if (a.vars.get(key) !== b.vars.get(key)) return false + if (!holdsReference(a, key) && !holdsReference(b, key)) return true + return a.verbatim.get(key) === b.verbatim.get(key) +} + +/** Why doctor cannot copy `key` out of `file` for the user, or `undefined` when it can. */ +function mirrorBlocker(file: EnvFile, key: string, value: string): string | undefined { + if (holdsReference(file, key)) { + return 'it contains a $ reference, so copy it and the variables it uses by hand' + } + const refusal = envValueRefusal(value) + return refusal && `${refusal}, so copy it by hand` +} + export type CheckGroup = 'files' | 'schema' | 'consistency' | 'coherence' | 'live' export type CheckStatus = 'pass' | 'warn' | 'fail' | 'skip' @@ -144,7 +174,9 @@ function checkFiles(ctx: CheckContext): Finding[] { const value = ctx.env.sim.vars.get(key) // Skip placeholders so seeding never copies an .env.example stub // into the new file (matches autofixForMissing). - if (value && !isPlaceholder(value)) values[key] = value + if (value && !isPlaceholder(value) && !mirrorBlocker(ctx.env.sim, key, value)) { + values[key] = value + } } writeEnvValues(target, values) } @@ -166,6 +198,8 @@ function autofixForMissing( simValue && !isPlaceholder(simValue) ) { + // A generated secret would differ from the app's, so a value doctor can't copy gets no fix. + if (mirrorBlocker(ctx.env.sim, key, simValue)) return undefined return () => writeEnvValues(target, { [key]: simValue }) } if (MIN_32_KEYS.has(key)) { @@ -195,16 +229,16 @@ function checkSchema(ctx: CheckContext): Finding[] { continue } if (isPlaceholder(value)) { + // Same fix as a missing key: a shared secret is mirrored from the app, never regenerated. + const autofix = MIN_32_KEYS.has(key) ? autofixForMissing(ctx, target, key) : undefined findings.push({ group: 'schema', status: production ? 'fail' : 'warn', message: `${rel(file)}: ${key} still has the .env.example placeholder`, - fix: MIN_32_KEYS.has(key) - ? 'doctor --fix generates a real value' + fix: autofix + ? 'doctor --fix sets a real value' : `replace the placeholder in ${rel(file)}`, - autofix: MIN_32_KEYS.has(key) - ? () => writeEnvValues(target, { [key]: generateSecret() }) - : undefined, + autofix, }) continue } @@ -262,13 +296,16 @@ function checkConsistency(ctx: CheckContext): Finding[] { const simValue = sim.vars.get(key) const realtimeValue = realtime.vars.get(key) if (!simValue || !realtimeValue) continue - if (simValue !== realtimeValue) { + if (!sameEnvValue(sim, realtime, key)) { + const blocker = mirrorBlocker(sim, key, simValue) findings.push({ group: 'consistency', status: 'fail', message: `${key} differs between apps/sim/.env and apps/realtime/.env`, - fix: 'doctor --fix mirrors the apps/sim/.env value', - autofix: () => writeEnvValues('realtime', { [key]: simValue }), + fix: blocker + ? `mirror the apps/sim/.env value: ${blocker}` + : 'doctor --fix mirrors the apps/sim/.env value', + autofix: blocker ? undefined : () => writeEnvValues('realtime', { [key]: simValue }), }) } } @@ -276,14 +313,17 @@ function checkConsistency(ctx: CheckContext): Finding[] { if (sim.exists && db.exists) { const simDsn = sim.vars.get('DATABASE_URL') const dbDsn = db.vars.get('DATABASE_URL') - if (simDsn && dbDsn && simDsn !== dbDsn) { + if (simDsn && dbDsn && !sameEnvValue(sim, db, 'DATABASE_URL')) { + const blocker = mirrorBlocker(sim, 'DATABASE_URL', simDsn) findings.push({ group: 'consistency', status: 'fail', message: 'DATABASE_URL differs between apps/sim/.env and packages/db/.env — migrations would hit a different database', - fix: 'doctor --fix mirrors the apps/sim/.env value', - autofix: () => writeEnvValues('db', { DATABASE_URL: simDsn }), + fix: blocker + ? `mirror the apps/sim/.env value: ${blocker}` + : 'doctor --fix mirrors the apps/sim/.env value', + autofix: blocker ? undefined : () => writeEnvValues('db', { DATABASE_URL: simDsn }), }) } } diff --git a/packages/sim-setup/src/env-files.ts b/packages/sim-setup/src/env-files.ts index e07f89746e8..1cbbe9ed0da 100644 --- a/packages/sim-setup/src/env-files.ts +++ b/packages/sim-setup/src/env-files.ts @@ -58,10 +58,22 @@ export interface EnvFile { exists: boolean content: string vars: Map + /** Each value as written in the file, quotes and escapes included. */ + verbatim: Map } const LINE_RE = /^\s*(?:export\s+)?([A-Za-z_][A-Za-z0-9_]*)\s*=\s*(.*)$/ +/** The value as written: a quoted value up to its closing quote, else up to an inline comment. */ +function verbatimValue(raw: string): string { + const trimmed = raw.trim() + if (trimmed.startsWith('"') || trimmed.startsWith("'")) { + const end = trimmed.indexOf(trimmed[0], 1) + return end === -1 ? trimmed : trimmed.slice(0, end + 1) + } + return trimmed.replace(/\s+#.*$/, '').trim() +} + function parseValue(raw: string): string { const trimmed = raw.trim() if (trimmed.startsWith('"') || trimmed.startsWith("'")) { @@ -86,7 +98,12 @@ export function readEnvFile(target: EnvTarget): EnvFile { const filePath = ENV_PATHS[target] const exists = existsSync(filePath) const content = exists ? readFileSync(filePath, 'utf8') : '' - return { target, path: filePath, exists, content, vars: parseEnv(content) } + const verbatim = new Map() + for (const line of content.split('\n')) { + const match = LINE_RE.exec(line) + if (match) verbatim.set(match[1], verbatimValue(match[2])) + } + return { target, path: filePath, exists, content, vars: parseEnv(content), verbatim } } /** @@ -96,24 +113,34 @@ export function readEnvFile(target: EnvTarget): EnvFile { * expand `$NAME` even inside single quotes, and the one spelling all four read as a * literal `$` is `\$` inside double quotes. */ -function formatEnvValue(key: string, value: string): string { - if (/[\r\n]/.test(value)) { - throw new Error(`${key} cannot be written to an environment file: it contains a line break`) - } +function spellEnvValue(value: string): { text: string } | { refusal: string } { + if (/[\r\n]/.test(value)) return { refusal: 'it contains a line break' } if (value.includes('$')) { if (/["\\`]/.test(value)) { - throw new Error( - `${key} cannot be written to an environment file: it mixes $ with a double quote, backslash, or backtick` - ) + return { refusal: 'it mixes $ with a double quote, backslash, or backtick' } } - return `"${value.replaceAll('$', '\\$')}"` + return { text: `"${value.replaceAll('$', '\\$')}"` } + } + if (value === value.trim() && !value.includes('#') && !/^["'`]/.test(value)) { + return { text: value } + } + if (!value.includes("'")) return { text: `'${value}'` } + if (!/["\\`]/.test(value)) return { text: `"${value}"` } + return { refusal: 'it mixes quotes with a # or edge whitespace' } +} + +/** Why `upsertEnv` cannot write `value` so every loader reads it back unchanged, if it cannot. */ +export function envValueRefusal(value: string): string | undefined { + const spelling = spellEnvValue(value) + return 'refusal' in spelling ? spelling.refusal : undefined +} + +function formatEnvValue(key: string, value: string): string { + const spelling = spellEnvValue(value) + if ('refusal' in spelling) { + throw new Error(`${key} cannot be written to an environment file: ${spelling.refusal}`) } - if (value === value.trim() && !value.includes('#') && !/^["'`]/.test(value)) return value - if (!value.includes("'")) return `'${value}'` - if (!/["\\`]/.test(value)) return `"${value}"` - throw new Error( - `${key} cannot be written to an environment file: it mixes quotes with a # or edge whitespace` - ) + return spelling.text } /** diff --git a/packages/sim-setup/src/lifecycle.ts b/packages/sim-setup/src/lifecycle.ts index 265d00d05ab..de013cf98b7 100644 --- a/packages/sim-setup/src/lifecycle.ts +++ b/packages/sim-setup/src/lifecycle.ts @@ -595,9 +595,14 @@ async function reset(install: Install | null): Promise { // With the daemon down nothing managed is visible, so a reset would archive the // env files while the volumes they unlock survive. if (install?.kind !== 'k8s' && executableExists('docker') && !dockerReachable()) { + // With no install detected, a Kubernetes install whose context isn't current looks the same. + const hints = [`start Docker, then re-run ${theme.command('npx sim-setup reset')}`] + if (!install) { + hints.push(`if Sim runs on Kubernetes, switch kubectl to its cluster's context and re-run`) + } throw new SetupError( "Docker isn't reachable, so the managed containers and volumes can't be removed. Your env files were left in place.", - [`start Docker, then re-run ${theme.command('npx sim-setup reset')}`] + hints ) } const ok = await p.confirm({