mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-08-21 06:32:16 +03:00
fix(cli): report .env lines that never take effect (#10870)
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
This commit is contained in:
@@ -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();
|
||||
|
||||
1
changelog.d/fixes/10870-cli-env-collision.md
Normal file
1
changelog.d/fixes/10870-cli-env-collision.md
Normal file
@@ -0,0 +1 @@
|
||||
- fix(cli): warn when a .env line never takes effect, and stop swallowing an unreadable .env (#10870)
|
||||
130
tests/unit/cli-env-collision.test.ts
Normal file
130
tests/unit/cli-env-collision.test.ts
Normal file
@@ -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<typeof layout>,
|
||||
extraEnv: Record<string, string> = {}
|
||||
) {
|
||||
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 });
|
||||
}
|
||||
});
|
||||
Reference in New Issue
Block a user