From a3bc53e612e72c60b9f95b2a208365605be69cf8 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Tue, 10 Nov 2026 16:41:37 -0800 Subject: [PATCH 1/4] fix(sim-setup): mirror env values as written so ${VAR} references survive doctor --fix --- packages/sim-setup/src/checks.test.ts | 76 ++++++++++++++++++++++- packages/sim-setup/src/checks.ts | 69 ++++++++++++++++++--- packages/sim-setup/src/env-files.ts | 87 +++++++++++++++++++++------ packages/sim-setup/src/lifecycle.ts | 7 ++- 4 files changed, 208 insertions(+), 31 deletions(-) diff --git a/packages/sim-setup/src/checks.test.ts b/packages/sim-setup/src/checks.test.ts index 6c19ca4e53c..22745addc19 100644 --- a/packages/sim-setup/src/checks.test.ts +++ b/packages/sim-setup/src/checks.test.ts @@ -1,5 +1,21 @@ -import { describe, expect, it } from 'vitest' -import { type CheckContext, runChecks } from './checks' +import { mkdirSync, readFileSync, rmSync, writeFileSync } from 'node:fs' +import path from 'node:path' +import { loadEnvConfig } from '@next/env' +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, EnvTarget } from './env-files' function envFile(target: EnvTarget, values: Record = {}): EnvFile { @@ -9,6 +25,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 +82,58 @@ 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'), + } + + 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 fix(groups: Parameters[1]) { + for (const finding of await runChecks(loadCheckContext(false), groups)) finding.autofix?.() + } + + function nextLoads(file: string, key: string): string | undefined { + const silent = { info: () => {}, error: () => {} } + try { + return loadEnvConfig(path.dirname(file), false, silent, true).parsedEnv?.[key] + } finally { + Reflect.deleteProperty(process.env, key) + } + } + + beforeEach(() => rmSync(fakeRoot, { recursive: true, force: true })) + afterAll(() => rmSync(fakeRoot, { recursive: true, force: true })) + + it.each([ + ['a differing value', 'DATABASE_URL=postgresql://other@localhost/sim\n'], + [ + 'a reference an earlier fix escaped', + 'DATABASE_URL="postgresql://\\${DB_USER}@localhost/sim"\n', + ], + ])('mirrors a hand-written ${VAR} reference as a reference over %s', async (_, dbFile) => { + const dsn = 'postgresql://${DB_USER}@localhost/sim' + writeFiles({ sim: `DB_USER=alice\nDATABASE_URL=${dsn}\n`, realtime: '', db: dbFile }) + + await fix(['consistency']) + + expect(readFileSync(files.db, 'utf8')).toBe(`DATABASE_URL=${dsn}\n`) + }) + + it('rewrites a single-quoted $ value so the app reads it literally', async () => { + writeFiles({ sim: "SMTP_PASSWORD='pa$$word$HOME'\n", realtime: '', db: '' }) + + await fix(['schema']) + + expect(nextLoads(files.sim, 'SMTP_PASSWORD')).toBe('pa$$word$HOME') + }) +}) diff --git a/packages/sim-setup/src/checks.ts b/packages/sim-setup/src/checks.ts index a129fee3bc1..24523425f1b 100644 --- a/packages/sim-setup/src/checks.ts +++ b/packages/sim-setup/src/checks.ts @@ -17,10 +17,12 @@ import { portOpen } from './detect' import { type EnvFile, type EnvTarget, + type EnvValue, generateSecret, isPlaceholder, isTruthy, isUsableSecret, + isWritableEnvValue, readEnvFile, SECRET_KEYS, SHARED_KEYS, @@ -30,6 +32,26 @@ import { import { httpHealth, pgProbe, redisPing } from './probes' import { FLAG_TWINS, LOGIN_PROVIDERS } from './twins' +/** + * A value copied out of `file` as it is written there, so a mirror keeps a `${VAR}` + * reference a reference instead of escaping it into a literal. + */ +function asWritten(file: EnvFile, key: string, value: string): EnvValue { + const verbatim = file.verbatim.get(key) + return verbatim === undefined ? value : { verbatim } +} + +/** + * 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 + const aWritten = a.verbatim.get(key) ?? '' + const bWritten = b.verbatim.get(key) ?? '' + return aWritten === bWritten || !(aWritten.includes('$') || bWritten.includes('$')) +} + export type CheckGroup = 'files' | 'schema' | 'consistency' | 'coherence' | 'live' export type CheckStatus = 'pass' | 'warn' | 'fail' | 'skip' @@ -139,12 +161,12 @@ function checkFiles(ctx: CheckContext): Finding[] { autofix: canSeed ? () => { const keys = target === 'db' ? ['DATABASE_URL'] : [...SHARED_KEYS] - const values: Record = {} + const values: Record = {} for (const key of keys) { 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)) values[key] = asWritten(ctx.env.sim, key, value) } writeEnvValues(target, values) } @@ -166,7 +188,7 @@ function autofixForMissing( simValue && !isPlaceholder(simValue) ) { - return () => writeEnvValues(target, { [key]: simValue }) + return () => writeEnvValues(target, { [key]: asWritten(ctx.env.sim, key, simValue) }) } if (MIN_32_KEYS.has(key)) { return () => writeEnvValues(target, { [key]: generateSecret() }) @@ -262,13 +284,13 @@ 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)) { 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 }), + autofix: () => writeEnvValues('realtime', { [key]: asWritten(sim, key, simValue) }), }) } } @@ -276,14 +298,15 @@ 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')) { 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 }), + autofix: () => + writeEnvValues('db', { DATABASE_URL: asWritten(sim, 'DATABASE_URL', simDsn) }), }) } } @@ -297,6 +320,32 @@ function checkConsistency(ctx: CheckContext): Finding[] { return findings } +/** + * Single quotes read literally in a shell and in Docker Compose, but Next and Bun + * still expand `$NAME` inside them, so the app and Compose see different values. + */ +function checkDollarQuoting(ctx: CheckContext): Finding[] { + const findings: Finding[] = [] + for (const target of layoutTargets(ctx.layout)) { + const file = ctx.env[target] + for (const [key, written] of file.verbatim) { + if (!written.startsWith("'") || !written.includes('$')) continue + const value = file.vars.get(key) ?? '' + const writable = isWritableEnvValue(value) + findings.push({ + group: 'schema', + status: 'warn', + message: `${rel(file)}: ${key} is single-quoted with a $, which the app expands but Docker Compose does not`, + fix: writable + ? 'doctor --fix rewrites it so every loader reads the $ literally' + : `rewrite ${key} without single quotes; escape each literal $ as \\$ inside double quotes`, + autofix: writable ? () => writeEnvValues(target, { [key]: value }) : undefined, + }) + } + } + return findings +} + function checkCoherence(ctx: CheckContext): Finding[] { const findings: Finding[] = [] const sim = ctx.primary @@ -426,7 +475,10 @@ function checkCoherence(ctx: CheckContext): Finding[] { message: 'DISABLE_AUTH is on in apps/sim/.env but not apps/realtime/.env — the socket server still enforces auth, so the canvas breaks silently', fix: 'doctor --fix mirrors it into apps/realtime/.env', - autofix: () => writeEnvValues('realtime', { DISABLE_AUTH: disableAuth as string }), + autofix: () => + writeEnvValues('realtime', { + DISABLE_AUTH: asWritten(sim, 'DISABLE_AUTH', disableAuth as string), + }), }) } @@ -699,6 +751,7 @@ export async function runChecks(ctx: CheckContext, groups?: CheckGroup[]): Promi ...checkFiles(ctx), ...checkSchema(ctx), ...checkConsistency(ctx), + ...checkDollarQuoting(ctx), ...checkCoherence(ctx), ] if (ctx.live) findings.push(...(await checkLive(ctx))) diff --git a/packages/sim-setup/src/env-files.ts b/packages/sim-setup/src/env-files.ts index e07f89746e8..9b788986ba3 100644 --- a/packages/sim-setup/src/env-files.ts +++ b/packages/sim-setup/src/env-files.ts @@ -58,10 +58,32 @@ export interface EnvFile { exists: boolean content: string vars: Map + /** Each value as written in the file, quotes and escapes included, for copying it verbatim. */ + verbatim: Map } +/** + * A value copied from another env file exactly as written there, so a `${VAR}` + * reference stays a reference and quoting is preserved rather than re-escaped. + */ +export interface VerbatimEnvValue { + verbatim: string +} + +export type EnvValue = string | VerbatimEnvValue + 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("'")) { @@ -82,11 +104,27 @@ export function parseEnv(content: string): Map { return vars } +function parseVerbatimEnv(content: string): Map { + const vars = new Map() + for (const line of content.split('\n')) { + const match = LINE_RE.exec(line) + if (match) vars.set(match[1], verbatimValue(match[2])) + } + return vars +} + 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) } + return { + target, + path: filePath, + exists, + content, + vars: parseEnv(content), + verbatim: parseVerbatimEnv(content), + } } /** @@ -96,31 +134,40 @@ 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' } +} + +/** Whether `upsertEnv` can write `value` so every loader reads it back unchanged. */ +export function isWritableEnvValue(value: string): boolean { + return 'text' in spellEnvValue(value) +} + +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 } /** * Sets a key in env-file content: replaces the active line, uncomments a * commented-out line, or appends. Returns the new content. */ -export function upsertEnv(content: string, key: string, value: string): string { +export function upsertEnv(content: string, key: string, value: EnvValue): string { const lines = content.split('\n') const activeRe = new RegExp(`^\\s*(?:export\\s+)?${key}\\s*=`) const commentedRe = new RegExp(`^#\\s*${key}\\s*=`) @@ -130,7 +177,7 @@ export function upsertEnv(content: string, key: string, value: string): string { } const activeIdx = activeIndexes[0] ?? -1 const idx = activeIdx !== -1 ? activeIdx : lines.findIndex((l) => commentedRe.test(l)) - const newLine = `${key}=${formatEnvValue(key, value)}` + const newLine = `${key}=${typeof value === 'string' ? formatEnvValue(key, value) : value.verbatim}` if (idx === -1) { const trailing = lines.length > 0 && lines[lines.length - 1] === '' if (trailing) lines.splice(lines.length - 1, 0, newLine) @@ -145,7 +192,7 @@ export function upsertEnv(content: string, key: string, value: string): string { export function reconcileEnvContent( content: string, remove: readonly string[], - values: Record + values: Record ): string { const replacementKeys = new Set(Object.keys(values)) const removalKeys = new Set(remove.filter((key) => !replacementKeys.has(key))) @@ -166,7 +213,7 @@ export function reconcileEnvContent( export function reconcileEnvValues( target: EnvTarget, remove: readonly string[], - values: Record + values: Record ): void { const filePath = ENV_PATHS[target] let content: string @@ -185,7 +232,7 @@ export function writeEnvFile(filePath: string, contents: string): void { } /** Writes values into an env file, seeding a missing file from its .env.example. */ -export function writeEnvValues(target: EnvTarget, values: Record): void { +export function writeEnvValues(target: EnvTarget, values: Record): void { reconcileEnvValues(target, [], values) } 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({ From 41a8bdd41c04ddd423e80562335dacdc2b43eced Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Tue, 10 Nov 2026 16:52:15 -0800 Subject: [PATCH 2/4] fix(sim-setup): leave $ references for a hand copy instead of mirroring them, drop the single-quote rewrite --- packages/sim-setup/src/checks.test.ts | 53 +++++++++--------- packages/sim-setup/src/checks.ts | 78 ++++++++++---------------- packages/sim-setup/src/env-files.ts | 80 +++++++++------------------ 3 files changed, 79 insertions(+), 132 deletions(-) diff --git a/packages/sim-setup/src/checks.test.ts b/packages/sim-setup/src/checks.test.ts index 22745addc19..b5811fce7cc 100644 --- a/packages/sim-setup/src/checks.test.ts +++ b/packages/sim-setup/src/checks.test.ts @@ -1,6 +1,5 @@ import { mkdirSync, readFileSync, rmSync, writeFileSync } from 'node:fs' import path from 'node:path' -import { loadEnvConfig } from '@next/env' import { afterAll, beforeEach, describe, expect, it, vi } from 'vitest' const { fakeRoot } = vi.hoisted(() => { @@ -89,6 +88,7 @@ describe('doctor --fix on env files', () => { 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)) { @@ -98,42 +98,41 @@ describe('doctor --fix on env files', () => { } } - async function fix(groups: Parameters[1]) { - for (const finding of await runChecks(loadCheckContext(false), groups)) finding.autofix?.() - } - - function nextLoads(file: string, key: string): string | undefined { - const silent = { info: () => {}, error: () => {} } - try { - return loadEnvConfig(path.dirname(file), false, silent, true).parsedEnv?.[key] - } finally { - Reflect.deleteProperty(process.env, key) - } + 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 differing value', 'DATABASE_URL=postgresql://other@localhost/sim\n'], + ['a different URL', 'DATABASE_URL=postgresql://other@localhost/sim\n'], [ - 'a reference an earlier fix escaped', + 'the reference escaped into a literal', 'DATABASE_URL="postgresql://\\${DB_USER}@localhost/sim"\n', ], - ])('mirrors a hand-written ${VAR} reference as a reference over %s', async (_, dbFile) => { - const dsn = 'postgresql://${DB_USER}@localhost/sim' - writeFiles({ sim: `DB_USER=alice\nDATABASE_URL=${dsn}\n`, realtime: '', db: dbFile }) - - await fix(['consistency']) - - expect(readFileSync(files.db, 'utf8')).toBe(`DATABASE_URL=${dsn}\n`) - }) - - it('rewrites a single-quoted $ value so the app reads it literally', async () => { - writeFiles({ sim: "SMTP_PASSWORD='pa$$word$HOME'\n", realtime: '', db: '' }) + ])( + '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) + } + ) - await fix(['schema']) + 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(nextLoads(files.sim, 'SMTP_PASSWORD')).toBe('pa$$word$HOME') + expect(await fixConsistency()).toBeUndefined() }) }) diff --git a/packages/sim-setup/src/checks.ts b/packages/sim-setup/src/checks.ts index 24523425f1b..3b4bf7ad4f8 100644 --- a/packages/sim-setup/src/checks.ts +++ b/packages/sim-setup/src/checks.ts @@ -17,12 +17,10 @@ import { portOpen } from './detect' import { type EnvFile, type EnvTarget, - type EnvValue, generateSecret, isPlaceholder, isTruthy, isUsableSecret, - isWritableEnvValue, readEnvFile, SECRET_KEYS, SHARED_KEYS, @@ -33,12 +31,13 @@ import { httpHealth, pgProbe, redisPing } from './probes' import { FLAG_TWINS, LOGIN_PROVIDERS } from './twins' /** - * A value copied out of `file` as it is written there, so a mirror keeps a `${VAR}` - * reference a reference instead of escaping it into a literal. + * 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 asWritten(file: EnvFile, key: string, value: string): EnvValue { - const verbatim = file.verbatim.get(key) - return verbatim === undefined ? value : { verbatim } +function holdsReference(file: EnvFile, key: string): boolean { + return /(^|[^\\])\$/.test(file.verbatim.get(key) ?? '') } /** @@ -47,11 +46,12 @@ function asWritten(file: EnvFile, key: string, value: string): EnvValue { */ function sameEnvValue(a: EnvFile, b: EnvFile, key: string): boolean { if (a.vars.get(key) !== b.vars.get(key)) return false - const aWritten = a.verbatim.get(key) ?? '' - const bWritten = b.verbatim.get(key) ?? '' - return aWritten === bWritten || !(aWritten.includes('$') || bWritten.includes('$')) + if (!holdsReference(a, key) && !holdsReference(b, key)) return true + return a.verbatim.get(key) === b.verbatim.get(key) } +const MANUAL_MIRROR = 'it contains a $ reference, so copy it and the variables it uses by hand' + export type CheckGroup = 'files' | 'schema' | 'consistency' | 'coherence' | 'live' export type CheckStatus = 'pass' | 'warn' | 'fail' | 'skip' @@ -161,12 +161,14 @@ function checkFiles(ctx: CheckContext): Finding[] { autofix: canSeed ? () => { const keys = target === 'db' ? ['DATABASE_URL'] : [...SHARED_KEYS] - const values: Record = {} + const values: Record = {} for (const key of keys) { 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] = asWritten(ctx.env.sim, key, value) + if (value && !isPlaceholder(value) && !holdsReference(ctx.env.sim, key)) { + values[key] = value + } } writeEnvValues(target, values) } @@ -186,9 +188,10 @@ function autofixForMissing( target !== 'sim' && (SHARED_KEYS as readonly string[]).includes(key) && simValue && - !isPlaceholder(simValue) + !isPlaceholder(simValue) && + !holdsReference(ctx.env.sim, key) ) { - return () => writeEnvValues(target, { [key]: asWritten(ctx.env.sim, key, simValue) }) + return () => writeEnvValues(target, { [key]: simValue }) } if (MIN_32_KEYS.has(key)) { return () => writeEnvValues(target, { [key]: generateSecret() }) @@ -285,12 +288,15 @@ function checkConsistency(ctx: CheckContext): Finding[] { const realtimeValue = realtime.vars.get(key) if (!simValue || !realtimeValue) continue if (!sameEnvValue(sim, realtime, key)) { + const manual = holdsReference(sim, key) 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]: asWritten(sim, key, simValue) }), + fix: manual + ? `mirror the apps/sim/.env value: ${MANUAL_MIRROR}` + : 'doctor --fix mirrors the apps/sim/.env value', + autofix: manual ? undefined : () => writeEnvValues('realtime', { [key]: simValue }), }) } } @@ -299,14 +305,16 @@ function checkConsistency(ctx: CheckContext): Finding[] { const simDsn = sim.vars.get('DATABASE_URL') const dbDsn = db.vars.get('DATABASE_URL') if (simDsn && dbDsn && !sameEnvValue(sim, db, 'DATABASE_URL')) { + const manual = holdsReference(sim, 'DATABASE_URL') 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: asWritten(sim, 'DATABASE_URL', simDsn) }), + fix: manual + ? `mirror the apps/sim/.env value: ${MANUAL_MIRROR}` + : 'doctor --fix mirrors the apps/sim/.env value', + autofix: manual ? undefined : () => writeEnvValues('db', { DATABASE_URL: simDsn }), }) } } @@ -320,32 +328,6 @@ function checkConsistency(ctx: CheckContext): Finding[] { return findings } -/** - * Single quotes read literally in a shell and in Docker Compose, but Next and Bun - * still expand `$NAME` inside them, so the app and Compose see different values. - */ -function checkDollarQuoting(ctx: CheckContext): Finding[] { - const findings: Finding[] = [] - for (const target of layoutTargets(ctx.layout)) { - const file = ctx.env[target] - for (const [key, written] of file.verbatim) { - if (!written.startsWith("'") || !written.includes('$')) continue - const value = file.vars.get(key) ?? '' - const writable = isWritableEnvValue(value) - findings.push({ - group: 'schema', - status: 'warn', - message: `${rel(file)}: ${key} is single-quoted with a $, which the app expands but Docker Compose does not`, - fix: writable - ? 'doctor --fix rewrites it so every loader reads the $ literally' - : `rewrite ${key} without single quotes; escape each literal $ as \\$ inside double quotes`, - autofix: writable ? () => writeEnvValues(target, { [key]: value }) : undefined, - }) - } - } - return findings -} - function checkCoherence(ctx: CheckContext): Finding[] { const findings: Finding[] = [] const sim = ctx.primary @@ -475,10 +457,7 @@ function checkCoherence(ctx: CheckContext): Finding[] { message: 'DISABLE_AUTH is on in apps/sim/.env but not apps/realtime/.env — the socket server still enforces auth, so the canvas breaks silently', fix: 'doctor --fix mirrors it into apps/realtime/.env', - autofix: () => - writeEnvValues('realtime', { - DISABLE_AUTH: asWritten(sim, 'DISABLE_AUTH', disableAuth as string), - }), + autofix: () => writeEnvValues('realtime', { DISABLE_AUTH: disableAuth as string }), }) } @@ -751,7 +730,6 @@ export async function runChecks(ctx: CheckContext, groups?: CheckGroup[]): Promi ...checkFiles(ctx), ...checkSchema(ctx), ...checkConsistency(ctx), - ...checkDollarQuoting(ctx), ...checkCoherence(ctx), ] if (ctx.live) findings.push(...(await checkLive(ctx))) diff --git a/packages/sim-setup/src/env-files.ts b/packages/sim-setup/src/env-files.ts index 9b788986ba3..3f6993af275 100644 --- a/packages/sim-setup/src/env-files.ts +++ b/packages/sim-setup/src/env-files.ts @@ -58,20 +58,10 @@ export interface EnvFile { exists: boolean content: string vars: Map - /** Each value as written in the file, quotes and escapes included, for copying it verbatim. */ + /** Each value as written in the file, quotes and escapes included. */ verbatim: Map } -/** - * A value copied from another env file exactly as written there, so a `${VAR}` - * reference stays a reference and quoting is preserved rather than re-escaped. - */ -export interface VerbatimEnvValue { - verbatim: string -} - -export type EnvValue = string | VerbatimEnvValue - 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. */ @@ -104,27 +94,16 @@ export function parseEnv(content: string): Map { return vars } -function parseVerbatimEnv(content: string): Map { - const vars = new Map() - for (const line of content.split('\n')) { - const match = LINE_RE.exec(line) - if (match) vars.set(match[1], verbatimValue(match[2])) - } - return vars -} - 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), - verbatim: parseVerbatimEnv(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 } } /** @@ -134,40 +113,31 @@ 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 spellEnvValue(value: string): { text: string } | { refusal: string } { - if (/[\r\n]/.test(value)) return { refusal: 'it contains a line break' } +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`) + } if (value.includes('$')) { if (/["\\`]/.test(value)) { - return { refusal: 'it mixes $ with a double quote, backslash, or backtick' } + throw new Error( + `${key} cannot be written to an environment file: it mixes $ with a double quote, backslash, or backtick` + ) } - 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' } -} - -/** Whether `upsertEnv` can write `value` so every loader reads it back unchanged. */ -export function isWritableEnvValue(value: string): boolean { - return 'text' in spellEnvValue(value) -} - -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}`) + return `"${value.replaceAll('$', '\\$')}"` } - return spelling.text + 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` + ) } /** * Sets a key in env-file content: replaces the active line, uncomments a * commented-out line, or appends. Returns the new content. */ -export function upsertEnv(content: string, key: string, value: EnvValue): string { +export function upsertEnv(content: string, key: string, value: string): string { const lines = content.split('\n') const activeRe = new RegExp(`^\\s*(?:export\\s+)?${key}\\s*=`) const commentedRe = new RegExp(`^#\\s*${key}\\s*=`) @@ -177,7 +147,7 @@ export function upsertEnv(content: string, key: string, value: EnvValue): string } const activeIdx = activeIndexes[0] ?? -1 const idx = activeIdx !== -1 ? activeIdx : lines.findIndex((l) => commentedRe.test(l)) - const newLine = `${key}=${typeof value === 'string' ? formatEnvValue(key, value) : value.verbatim}` + const newLine = `${key}=${formatEnvValue(key, value)}` if (idx === -1) { const trailing = lines.length > 0 && lines[lines.length - 1] === '' if (trailing) lines.splice(lines.length - 1, 0, newLine) @@ -192,7 +162,7 @@ export function upsertEnv(content: string, key: string, value: EnvValue): string export function reconcileEnvContent( content: string, remove: readonly string[], - values: Record + values: Record ): string { const replacementKeys = new Set(Object.keys(values)) const removalKeys = new Set(remove.filter((key) => !replacementKeys.has(key))) @@ -213,7 +183,7 @@ export function reconcileEnvContent( export function reconcileEnvValues( target: EnvTarget, remove: readonly string[], - values: Record + values: Record ): void { const filePath = ENV_PATHS[target] let content: string @@ -232,7 +202,7 @@ export function writeEnvFile(filePath: string, contents: string): void { } /** Writes values into an env file, seeding a missing file from its .env.example. */ -export function writeEnvValues(target: EnvTarget, values: Record): void { +export function writeEnvValues(target: EnvTarget, values: Record): void { reconcileEnvValues(target, [], values) } From 6243955b7814fb9c76d8dd92720861a24bf6e121 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Tue, 10 Nov 2026 17:03:10 -0800 Subject: [PATCH 3/4] fix(sim-setup): never generate a shared secret doctor can't copy, and report unwritable values instead of aborting --- packages/sim-setup/src/checks.test.ts | 57 ++++++++++++++++++--------- packages/sim-setup/src/checks.ts | 33 ++++++++++------ packages/sim-setup/src/env-files.ts | 38 +++++++++++------- 3 files changed, 84 insertions(+), 44 deletions(-) diff --git a/packages/sim-setup/src/checks.test.ts b/packages/sim-setup/src/checks.test.ts index b5811fce7cc..2fb2e44f4d6 100644 --- a/packages/sim-setup/src/checks.test.ts +++ b/packages/sim-setup/src/checks.test.ts @@ -88,7 +88,7 @@ describe('doctor --fix on env files', () => { realtime: path.join(fakeRoot, 'apps/realtime/.env'), db: path.join(fakeRoot, 'packages/db/.env'), } - const referencingDsn = 'postgresql://${DB_USER}@localhost/sim' + const referencingDsn = `postgresql://\${DB_USER}@localhost/sim` function writeFiles(contents: Record) { for (const [target, content] of Object.entries(contents)) { @@ -111,25 +111,46 @@ describe('doctor --fix on env files', () => { ['a different URL', 'DATABASE_URL=postgresql://other@localhost/sim\n'], [ 'the reference escaped into a literal', - 'DATABASE_URL="postgresql://\\${DB_USER}@localhost/sim"\n', + `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) - } - ) + ])('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('leaves a shared secret it cannot copy unset rather than generating a different one', async () => { + const secret = `BETTER_AUTH_SECRET=\${AUTH_SECRET_BASE}0123456789abcdef0123456789abcdef` + writeFiles({ + sim: `${secret}\n`, + realtime: 'DATABASE_URL=postgresql://localhost/sim\n', + db: '', + }) + + for (const finding of await runChecks(loadCheckContext(false), ['schema'])) finding.autofix?.() + + expect(readFileSync(files.realtime, 'utf8')).not.toContain('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 () => { + 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 }) diff --git a/packages/sim-setup/src/checks.ts b/packages/sim-setup/src/checks.ts index 3b4bf7ad4f8..8305685c5a8 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, @@ -50,7 +51,14 @@ function sameEnvValue(a: EnvFile, b: EnvFile, key: string): boolean { return a.verbatim.get(key) === b.verbatim.get(key) } -const MANUAL_MIRROR = 'it contains a $ reference, so copy it and the variables it uses by hand' +/** 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' @@ -166,7 +174,7 @@ 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) && !holdsReference(ctx.env.sim, key)) { + if (value && !isPlaceholder(value) && !mirrorBlocker(ctx.env.sim, key, value)) { values[key] = value } } @@ -188,9 +196,10 @@ function autofixForMissing( target !== 'sim' && (SHARED_KEYS as readonly string[]).includes(key) && simValue && - !isPlaceholder(simValue) && - !holdsReference(ctx.env.sim, key) + !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)) { @@ -288,15 +297,15 @@ function checkConsistency(ctx: CheckContext): Finding[] { const realtimeValue = realtime.vars.get(key) if (!simValue || !realtimeValue) continue if (!sameEnvValue(sim, realtime, key)) { - const manual = holdsReference(sim, 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: manual - ? `mirror the apps/sim/.env value: ${MANUAL_MIRROR}` + fix: blocker + ? `mirror the apps/sim/.env value: ${blocker}` : 'doctor --fix mirrors the apps/sim/.env value', - autofix: manual ? undefined : () => writeEnvValues('realtime', { [key]: simValue }), + autofix: blocker ? undefined : () => writeEnvValues('realtime', { [key]: simValue }), }) } } @@ -305,16 +314,16 @@ function checkConsistency(ctx: CheckContext): Finding[] { const simDsn = sim.vars.get('DATABASE_URL') const dbDsn = db.vars.get('DATABASE_URL') if (simDsn && dbDsn && !sameEnvValue(sim, db, 'DATABASE_URL')) { - const manual = holdsReference(sim, '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: manual - ? `mirror the apps/sim/.env value: ${MANUAL_MIRROR}` + fix: blocker + ? `mirror the apps/sim/.env value: ${blocker}` : 'doctor --fix mirrors the apps/sim/.env value', - autofix: manual ? undefined : () => writeEnvValues('db', { DATABASE_URL: simDsn }), + 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 3f6993af275..1cbbe9ed0da 100644 --- a/packages/sim-setup/src/env-files.ts +++ b/packages/sim-setup/src/env-files.ts @@ -113,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 } /** From acca16f3ef2b7ff380f834e9572be38b2cda2bcd Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Tue, 10 Nov 2026 17:13:02 -0800 Subject: [PATCH 4/4] fix(sim-setup): give a placeholder shared secret the same fix as a missing one --- packages/sim-setup/src/checks.test.ts | 30 +++++++++++++++------------ packages/sim-setup/src/checks.ts | 10 ++++----- 2 files changed, 22 insertions(+), 18 deletions(-) diff --git a/packages/sim-setup/src/checks.test.ts b/packages/sim-setup/src/checks.test.ts index 2fb2e44f4d6..9157d66bbb8 100644 --- a/packages/sim-setup/src/checks.test.ts +++ b/packages/sim-setup/src/checks.test.ts @@ -15,7 +15,7 @@ vi.mock('./context', async (importOriginal) => { }) import { type CheckContext, loadCheckContext, runChecks } from './checks' -import type { EnvFile, EnvTarget } from './env-files' +import { type EnvFile, type EnvTarget, parseEnv } from './env-files' function envFile(target: EnvTarget, values: Record = {}): EnvFile { return { @@ -126,18 +126,22 @@ describe('doctor --fix on env files', () => { expect(readFileSync(files.db, 'utf8')).toBe(dbFile) }) - it('leaves a shared secret it cannot copy unset rather than generating a different one', async () => { - const secret = `BETTER_AUTH_SECRET=\${AUTH_SECRET_BASE}0123456789abcdef0123456789abcdef` - writeFiles({ - sim: `${secret}\n`, - realtime: 'DATABASE_URL=postgresql://localhost/sim\n', - db: '', - }) - - for (const finding of await runChecks(loadCheckContext(false), ['schema'])) finding.autofix?.() - - expect(readFileSync(files.realtime, 'utf8')).not.toContain('BETTER_AUTH_SECRET') - }) + 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' diff --git a/packages/sim-setup/src/checks.ts b/packages/sim-setup/src/checks.ts index 8305685c5a8..b6474b1a961 100644 --- a/packages/sim-setup/src/checks.ts +++ b/packages/sim-setup/src/checks.ts @@ -229,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 }