From dacf4c3c1af6b11305f0d6683acf47d3c2abb56a Mon Sep 17 00:00:00 2001 From: Dizzle <112548150+maxmad64bis@users.noreply.github.com> Date: Thu, 20 Aug 2026 20:47:15 +0200 Subject: [PATCH] fix(cli): report .env lines that never take effect (#10870) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Obrigado — resolve o cenário real do #6194: uma linha de .env que o shell já tinha exportado antes (ex.: HOSTNAME=0.0.0.0) era silenciosamente ignorada pelo loader first-wins, sem nenhum aviso — o servidor bindava no hostname da máquina, localhost parava de responder, e ModelSync/health checks falhavam com ECONNREFUSED sem pista nenhuma. Agora cada chave mascarada emite um warning em stderr (nome da chave + as duas origens, nunca o valor); um .env ilegível também vira warning em vez de falha silenciosa no boot. Validação (worktree combinado a partir de origin/release/v3.8.50, 0 conflitos): - typecheck:core limpo, complexity/cognitive-complexity dentro do baseline - tests/unit/cli-env-collision.test.ts — 4/4 passando (3 falham no base) - Suítes CLI env vizinhas (cli-data-dir-env-loading, cli-env-inline-comment-10100, cli-data-dir-env, cli-entrypoint, cli-electron-to-cli-migration-server-env-7302, cli-storage-key-bootstrap) — intactas e verdes --- bin/omniroute.mjs | 19 ++- changelog.d/fixes/10870-cli-env-collision.md | 1 + tests/unit/cli-env-collision.test.ts | 130 +++++++++++++++++++ 3 files changed, 148 insertions(+), 2 deletions(-) create mode 100644 changelog.d/fixes/10870-cli-env-collision.md create mode 100644 tests/unit/cli-env-collision.test.ts diff --git a/bin/omniroute.mjs b/bin/omniroute.mjs index c023879da8..fb0a455520 100755 --- a/bin/omniroute.mjs +++ b/bin/omniroute.mjs @@ -119,6 +119,9 @@ function loadEnvFile() { addEnvPath(join(ROOT, ".env")); } + const keyOrigin = new Map(); + const shadowed = new Map(); + for (const envPath of envPaths) { try { if (existsSync(envPath)) { @@ -131,19 +134,31 @@ function loadEnvFile() { const key = trimmed.slice(0, eqIdx).trim(); if (process.env[key] === undefined) { process.env[key] = parseEnvValue(trimmed.slice(eqIdx + 1)); + keyOrigin.set(key, envPath); + } else if (!shadowed.has(key)) { + // The line is inert: something set this key first. Report it once + // per key, whether the winner was an earlier file or the process + // environment (#6194: a shell's own HOSTNAME beat the .env and the + // server bound to the wrong address in silence). + shadowed.set(key, { winner: keyOrigin.get(key) ?? null, loser: envPath }); } } } loadedEnvPaths.push(envPath); } - } catch { - // Ignore errors reading env files. + } catch (err) { + console.warn(` \x1b[33m⚠ Could not read ${envPath}: ${err?.message ?? err}\x1b[0m`); } } for (const envPath of loadedEnvPaths) { console.log(` \x1b[2m📋 Loaded env from ${envPath}\x1b[0m`); } + + for (const [key, { winner, loser }] of shadowed) { + const setter = winner ? winner : "the environment"; + console.warn(` \x1b[33m⚠ ${key} in ${loser} is ignored, ${setter} set it first\x1b[0m`); + } } loadEnvFile(); diff --git a/changelog.d/fixes/10870-cli-env-collision.md b/changelog.d/fixes/10870-cli-env-collision.md new file mode 100644 index 0000000000..95a428ba08 --- /dev/null +++ b/changelog.d/fixes/10870-cli-env-collision.md @@ -0,0 +1 @@ +- fix(cli): warn when a .env line never takes effect, and stop swallowing an unreadable .env (#10870) diff --git a/tests/unit/cli-env-collision.test.ts b/tests/unit/cli-env-collision.test.ts new file mode 100644 index 0000000000..cb11ac4474 --- /dev/null +++ b/tests/unit/cli-env-collision.test.ts @@ -0,0 +1,130 @@ +import test from "node:test"; +import assert from "node:assert/strict"; +import { spawnSync } from "node:child_process"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { fileURLToPath } from "node:url"; + +const ROOT = path.resolve(path.dirname(fileURLToPath(import.meta.url)), "..", ".."); +const BIN = path.join(ROOT, "bin", "omniroute.mjs"); + +function layout() { + const tmp = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-cli-env-collision-")); + const home = path.join(tmp, "home"); + const dataDir = path.join(tmp, "data"); + const cwd = path.join(tmp, "cwd"); + const appDataDir = + process.platform === "win32" + ? path.join(tmp, "appdata", "omniroute") + : path.join(home, ".omniroute"); + fs.mkdirSync(dataDir, { recursive: true }); + fs.mkdirSync(appDataDir, { recursive: true }); + fs.mkdirSync(cwd, { recursive: true }); + return { tmp, home, dataDir, cwd }; +} + +function runCli( + { tmp, home, dataDir, cwd }: ReturnType, + extraEnv: Record = {} +) { + const cleanEnv = { ...process.env }; + for (const key of ["OMNIROUTE_BASE_URL", "PORT", "STORAGE_ENCRYPTION_KEY"]) { + delete cleanEnv[key]; + } + return spawnSync("node", [BIN, "env", "show", "--json"], { + cwd, + env: { + ...cleanEnv, + DATA_DIR: dataDir, + HOME: home, + USERPROFILE: home, + APPDATA: path.join(tmp, "appdata"), + CI: "1", + OMNIROUTE_CLI_SKIP_REPO_ENV: "1", + OMNIROUTE_NO_UPDATE_NOTIFIER: "1", + ...extraEnv, + }, + encoding: "utf-8", + timeout: 60_000, + }); +} + +test("a key masked by an earlier .env is named, with both files and without its value", () => { + const dirs = layout(); + try { + fs.writeFileSync( + path.join(dirs.dataDir, ".env"), + "OMNIROUTE_BASE_URL=https://data.example/v1\n" + ); + fs.writeFileSync(path.join(dirs.cwd, ".env"), "OMNIROUTE_BASE_URL=https://cwd.example/v1\n"); + + const stderr = runCli(dirs).stderr ?? ""; + + assert.match(stderr, /OMNIROUTE_BASE_URL/); + assert.ok(stderr.includes(path.join(dirs.cwd, ".env")), `ignored file named: ${stderr}`); + assert.ok(stderr.includes(path.join(dirs.dataDir, ".env")), `winning file named: ${stderr}`); + assert.ok(!stderr.includes("cwd.example"), "the ignored value must never be printed"); + assert.ok(!stderr.includes("data.example"), "the winning value must never be printed"); + } finally { + fs.rmSync(dirs.tmp, { recursive: true, force: true }); + } +}); + +test("a key each file declares once says nothing", () => { + const dirs = layout(); + try { + fs.writeFileSync( + path.join(dirs.dataDir, ".env"), + "OMNIROUTE_BASE_URL=https://data.example/v1\n" + ); + fs.writeFileSync(path.join(dirs.cwd, ".env"), "PORT=34567\n"); + + const stderr = runCli(dirs).stderr ?? ""; + assert.ok(!/OMNIROUTE_BASE_URL|PORT/.test(stderr), `nothing to report: ${stderr}`); + } finally { + fs.rmSync(dirs.tmp, { recursive: true, force: true }); + } +}); + +test("a key the environment already set is reported too — that is #6194", () => { + const dirs = layout(); + try { + fs.writeFileSync( + path.join(dirs.dataDir, ".env"), + "OMNIROUTE_BASE_URL=https://data.example/v1\n" + ); + + const stderr = runCli(dirs, { OMNIROUTE_BASE_URL: "https://shell.example/v1" }).stderr ?? ""; + + assert.match(stderr, /OMNIROUTE_BASE_URL/); + assert.ok(stderr.includes(path.join(dirs.dataDir, ".env")), `inert file named: ${stderr}`); + assert.match(stderr, /environment/); + assert.ok(!stderr.includes("shell.example"), "the winning value must never be printed"); + assert.ok(!stderr.includes("data.example"), "the ignored value must never be printed"); + } finally { + fs.rmSync(dirs.tmp, { recursive: true, force: true }); + } +}); + +test("an unreadable .env is reported instead of being swallowed", () => { + const dirs = layout(); + try { + // A directory named `.env` passes existsSync and makes readFileSync throw + // EISDIR for any user, root included — unlike chmod 000. + fs.mkdirSync(path.join(dirs.cwd, ".env"), { recursive: true }); + fs.writeFileSync( + path.join(dirs.dataDir, ".env"), + "OMNIROUTE_BASE_URL=https://data.example/v1\n" + ); + + const result = runCli(dirs); + assert.equal(result.status, 0, "an unreadable .env must stay non-fatal"); + assert.ok( + (result.stderr ?? "").includes(path.join(dirs.cwd, ".env")), + `the unreadable file should be named: ${result.stderr}` + ); + } finally { + fs.rmSync(dirs.tmp, { recursive: true, force: true }); + } +});