diff --git a/bin/cli/utils/volatileEnvPath.mjs b/bin/cli/utils/volatileEnvPath.mjs new file mode 100644 index 0000000000..482b1ebca8 --- /dev/null +++ b/bin/cli/utils/volatileEnvPath.mjs @@ -0,0 +1,37 @@ +import { sep } from "node:path"; + +/** + * A `.env` inside the installed package directory does not survive an update: + * `npm i -g` replaces that directory wholesale, and postinstall recreates the + * file from `.env.example`. The CLI announces every env file it loads without + * distinguishing the ones that last from the one that doesn't. + * + * Returns the warning to print, or null when there is nothing worth saying. + * + * Two conditions, both required, so a development checkout never sees this: + * - the file sits inside the package root, and that root is inside a + * `node_modules` directory — i.e. an installed package, not a checkout, + * where the same path is stable and documented in SETUP_GUIDE.md; + * - the file actually supplied at least one value. First writer wins, so a + * file entirely shadowed by a durable one supplied nothing, and losing it + * costs nothing. + * + * @param {{ envPath: string, packageRoot: string, durableEnvPath: string, suppliedKeys: boolean }} args + * @returns {string | null} + */ +export function describeVolatileEnvWarning({ envPath, packageRoot, durableEnvPath, suppliedKeys }) { + if (!suppliedKeys) return null; + if (envPath === durableEnvPath) return null; + if (!isInsideInstalledPackage(packageRoot)) return null; + if (!envPath.startsWith(packageRoot + sep)) return null; + + return ( + `${envPath} lives inside the installed package: updating OmniRoute replaces it. ` + + `Move the values you set to ${durableEnvPath}, which updates leave alone.` + ); +} + +/** True when the path sits under a `node_modules` directory. */ +function isInsideInstalledPackage(dir) { + return typeof dir === "string" && dir.split(sep).includes("node_modules"); +} diff --git a/bin/omniroute.mjs b/bin/omniroute.mjs index 09b133df4f..de51120643 100755 --- a/bin/omniroute.mjs +++ b/bin/omniroute.mjs @@ -29,6 +29,7 @@ import { getDefaultDataDir } from "./cli/data-dir.mjs"; import { shouldProvisionStorageKey } from "./cli/utils/storageKeyProvision.mjs"; import { isVersionFastPath } from "./cli/utils/versionFastPath.mjs"; import { parseEnvValue } from "./cli/utils/parseEnvValue.mjs"; +import { describeVolatileEnvWarning } from "./cli/utils/volatileEnvPath.mjs"; const __filename = fileURLToPath(import.meta.url); const __dirname = dirname(__filename); @@ -91,9 +92,7 @@ function migrateElectronServerEnv(dataDir) { const serverEnvPath = join(dataDir, "server.env"); if (existsSync(envPath) || !existsSync(serverEnvPath)) return; writeFileSync(envPath, readFileSync(serverEnvPath, "utf-8"), "utf-8"); - console.log( - ` \x1b[2m♻ Migrated Electron secrets from ${serverEnvPath} to ${envPath}\x1b[0m` - ); + console.log(` \x1b[2m♻ Migrated Electron secrets from ${serverEnvPath} to ${envPath}\x1b[0m`); } catch { // Ignore errors migrating server.env — fall back to normal env loading below. } @@ -164,6 +163,21 @@ function loadEnvFile() { const setter = winner ? winner : "the environment"; console.warn(` \x1b[33m⚠ ${key} in ${loser} is ignored, ${setter} set it first\x1b[0m`); } + + // The package directory is replaced by the next `npm i -g`, so a .env kept + // there is silently lost. Say so once, and only when that file actually + // supplied something. + const durableEnvPath = join(process.env.DATA_DIR || getDefaultDataDir(), ".env"); + const suppliedKeys = [...keyOrigin.values()].some((origin) => origin === join(ROOT, ".env")); + const volatileWarning = describeVolatileEnvWarning({ + envPath: join(ROOT, ".env"), + packageRoot: ROOT, + durableEnvPath, + suppliedKeys, + }); + if (volatileWarning && loadedEnvPaths.includes(join(ROOT, ".env"))) { + console.warn(` \x1b[33m⚠ ${volatileWarning}\x1b[0m`); + } } loadEnvFile(); @@ -247,16 +261,16 @@ if (shouldProvisionStorageKey(process.argv)) { const langEnv = process.env.OMNIROUTE_LANG; const chosen = langArg || langEnv; if (chosen) { - const { setLocale } = await import( - pathToFileURL(join(ROOT, "bin", "cli", "i18n.mjs")).href - ); + const { setLocale } = await import(pathToFileURL(join(ROOT, "bin", "cli", "i18n.mjs")).href); setLocale(chosen); } } // Register update notifier — checks npm once per 24h, notifies on exit via stderr. const _pkg = JSON.parse(readFileSync(join(ROOT, "package.json"), "utf8")); -const _notifier = updateNotifier ? updateNotifier({ pkg: _pkg, updateCheckInterval: 1000 * 60 * 60 * 24 }) : null; +const _notifier = updateNotifier + ? updateNotifier({ pkg: _pkg, updateCheckInterval: 1000 * 60 * 60 * 24 }) + : null; process.on("exit", () => { if (!_notifier || !_notifier.update) return; if (process.env.OMNIROUTE_NO_UPDATE_NOTIFIER) return; @@ -265,7 +279,15 @@ process.on("exit", () => { const outputIdx = process.argv.indexOf("--output"); const outputVal = outputIdx >= 0 ? process.argv[outputIdx + 1] : null; if (outputVal === "json" || outputVal === "jsonl" || outputVal === "csv") return; - if (process.argv.some((a) => a.startsWith("--output=json") || a.startsWith("--output=jsonl") || a.startsWith("--output=csv"))) return; + if ( + process.argv.some( + (a) => + a.startsWith("--output=json") || + a.startsWith("--output=jsonl") || + a.startsWith("--output=csv") + ) + ) + return; if (_notifier.update) { _notifier.notify({ defer: false, diff --git a/changelog.d/fixes/11437-cli-warn-volatile-package-env.md b/changelog.d/fixes/11437-cli-warn-volatile-package-env.md new file mode 100644 index 0000000000..876f5c6c8e --- /dev/null +++ b/changelog.d/fixes/11437-cli-warn-volatile-package-env.md @@ -0,0 +1 @@ +- **fix(cli):** the CLI now says when a loaded `.env` lives inside the installed package directory (#11437). It already announces every env file it reads, without distinguishing the ones that survive an update from the one that does not: `npm i -g` replaces the package directory wholesale, so values set there are gone at the next update, silently. The warning names the durable path to move them to, and fires only when that file actually supplied a value — a file entirely shadowed by a durable one supplied nothing. A development checkout stays silent: there the same path is stable and documented in `SETUP_GUIDE.md`. diff --git a/tests/unit/cli-volatile-env-path.test.ts b/tests/unit/cli-volatile-env-path.test.ts new file mode 100644 index 0000000000..dad4efbc6f --- /dev/null +++ b/tests/unit/cli-volatile-env-path.test.ts @@ -0,0 +1,82 @@ +/** + * The CLI announces every .env it loads. One of those locations is the + * installed package directory, which `npm i -g` replaces wholesale — so the + * file an operator edits there is gone at the next update, without a word. + * + * describeVolatileEnvWarning() decides when to say so. It must stay silent for + * a development checkout, where that same path is stable and documented, and + * for a file whose keys were all shadowed by a durable one — it supplied + * nothing, so losing it costs nothing. + */ + +import test from "node:test"; +import assert from "node:assert/strict"; +import path from "node:path"; +import { describeVolatileEnvWarning } from "../../bin/cli/utils/volatileEnvPath.mjs"; + +const INSTALLED_ROOT = path.join("/usr", "lib", "node_modules", "omniroute"); +const CHECKOUT_ROOT = path.join("/home", "dev", "OmniRoute"); +const DURABLE = path.join("/home", "dev", ".omniroute", ".env"); + +test("an installed package .env that supplied keys is reported as volatile", () => { + const message = describeVolatileEnvWarning({ + envPath: path.join(INSTALLED_ROOT, ".env"), + packageRoot: INSTALLED_ROOT, + durableEnvPath: DURABLE, + suppliedKeys: true, + }); + + assert.ok(message, "an installed package .env must be reported"); + assert.match(message, /update/i, "the message must say what destroys the file"); + assert.ok(message.includes(DURABLE), "the message must name the durable path to move to"); +}); + +test("a development checkout says nothing", () => { + // Same file name, stable location: `npm install` in a checkout preserves it, + // and SETUP_GUIDE.md documents it. Warning here would fire on every start. + assert.equal( + describeVolatileEnvWarning({ + envPath: path.join(CHECKOUT_ROOT, ".env"), + packageRoot: CHECKOUT_ROOT, + durableEnvPath: DURABLE, + suppliedKeys: true, + }), + null + ); +}); + +test("a file that supplied no key says nothing", () => { + assert.equal( + describeVolatileEnvWarning({ + envPath: path.join(INSTALLED_ROOT, ".env"), + packageRoot: INSTALLED_ROOT, + durableEnvPath: DURABLE, + suppliedKeys: false, + }), + null + ); +}); + +test("the durable file itself says nothing, wherever it sits", () => { + assert.equal( + describeVolatileEnvWarning({ + envPath: DURABLE, + packageRoot: INSTALLED_ROOT, + durableEnvPath: DURABLE, + suppliedKeys: true, + }), + null + ); +}); + +test("a path outside the package root says nothing", () => { + assert.equal( + describeVolatileEnvWarning({ + envPath: path.join("/srv", "app", ".env"), + packageRoot: INSTALLED_ROOT, + durableEnvPath: DURABLE, + suppliedKeys: true, + }), + null + ); +});