From b074c6d75ebb79e6030ae54a42d016bc6c47fdf9 Mon Sep 17 00:00:00 2001 From: "R. Beltran" Date: Sun, 5 Jul 2026 07:30:16 +0200 Subject: [PATCH] fix(cli): detect POSIX auto-set HOSTNAME via os.hostname() to fix bind address (#6194) (#6195) POSIX shells (bash/zsh) always set HOSTNAME to the machine name. The .env loader uses first-wins semantics, so HOSTNAME=0.0.0.0 in .env is silently ignored. This causes the server to bind to the LAN hostname instead of 0.0.0.0, breaking localhost access and all internal self-requests (ModelSync, HealthCheck, cloud sync). The fix compares process.env.HOSTNAME against os.hostname(): when they match, it's the POSIX auto-set signature and HOSTNAME is ignored. OMNIROUTE_SERVER_HOST takes precedence as the dedicated escape hatch. Backward compatibility is preserved: users who set HOSTNAME to a value that doesn't match the machine name (e.g. Windows CMD/PowerShell users with HOSTNAME in .env) will still have their value honoured. Closes #6194 --- .env.example | 5 ++ bin/cli/commands/serve.mjs | 13 ++++- docs/reference/ENVIRONMENT.md | 3 +- tests/unit/cli-serve-hostname.test.ts | 71 +++++++++++++++++++++------ 4 files changed, 75 insertions(+), 17 deletions(-) diff --git a/.env.example b/.env.example index fdfe5d0b0f..73b87e9d93 100644 --- a/.env.example +++ b/.env.example @@ -187,8 +187,13 @@ OMNIROUTE_USE_TURBOPACK=1 # Hostname/bind address for the Next.js server. # Used by: scripts/dev/run-next.mjs (HOST), Playwright runner (HOSTNAME). # Default: 0.0.0.0 (HOST) / 127.0.0.1 (HOSTNAME inside tests). +# NOTE: Do NOT use `HOSTNAME` — it is a POSIX shell variable automatically set to +# the machine name by bash/zsh. The .env loader cannot override it (first-wins +# semantics). Use OMNIROUTE_SERVER_HOST instead for `omniroute serve`. +# See: https://github.com/diegosouzapw/OmniRoute/issues/6194 #HOST=0.0.0.0 #HOSTNAME=127.0.0.1 +#OMNIROUTE_SERVER_HOST=0.0.0.0 # Environment mode — affects Next.js behavior, logging verbosity, and caching. # Values: production | development | Default: production diff --git a/bin/cli/commands/serve.mjs b/bin/cli/commands/serve.mjs index 9ba6579c3f..ab9d793ca3 100644 --- a/bin/cli/commands/serve.mjs +++ b/bin/cli/commands/serve.mjs @@ -2,7 +2,7 @@ import { spawn } from "node:child_process"; import { existsSync, readFileSync } from "node:fs"; import { join, dirname } from "node:path"; import { fileURLToPath } from "node:url"; -import { platform, totalmem } from "node:os"; +import { platform, totalmem, hostname as osHostname } from "node:os"; import { t } from "../i18n.mjs"; import { writePidFile, cleanupPidFile, waitForServer } from "../utils/pid.mjs"; import { ServerSupervisor, detectMitmCrash } from "../runtime/processSupervisor.mjs"; @@ -170,7 +170,16 @@ export async function runServe(opts = {}) { PORT: String(dashboardPort), DASHBOARD_PORT: String(dashboardPort), API_PORT: String(apiPort), - HOSTNAME: process.env.HOSTNAME || "0.0.0.0", + // #6194: POSIX shells (bash/zsh) auto-set HOSTNAME to the machine name — the + // .env loader (first-wins) can never override it. Ignore HOSTNAME when it + // matches the OS-reported hostname (the auto-set signature). OMNIROUTE_SERVER_HOST + // takes precedence; legacy HOSTNAME values that don't match os.hostname() are + // still honoured for backward compatibility (e.g. Windows CMD/PowerShell users + // who set HOSTNAME in .env where it is NOT auto-set). + HOSTNAME: + process.env.OMNIROUTE_SERVER_HOST || + (process.env.HOSTNAME !== osHostname() ? process.env.HOSTNAME : undefined) || + "0.0.0.0", NODE_ENV: "production", // #5238: preserve a user-set NODE_OPTIONS (incl. their own // `--max-old-space-size=…`) instead of clobbering it with the calibrated diff --git a/docs/reference/ENVIRONMENT.md b/docs/reference/ENVIRONMENT.md index efe32cff43..e288136d00 100644 --- a/docs/reference/ENVIRONMENT.md +++ b/docs/reference/ENVIRONMENT.md @@ -139,7 +139,8 @@ OmniRoute uses **SQLite** (via `better-sqlite3`) for all persistence. These vari | `CREDENTIAL_HEALTH_CACHE_TTL` | `300000` | `open-sse/config/constants.ts` / `src/lib/credentialHealth/cache.ts` | TTL (ms) for cached credential health status. | | `OMNIROUTE_DISABLE_CREDENTIAL_HEALTH_CHECK` | `false` | `src/lib/credentialHealth/scheduler.ts` | Set to `1` or `true` to disable background periodic testing of provider connections. | | `HOST` | `0.0.0.0` | `scripts/dev/run-next.mjs` | Bind address for the Next.js dev/start server. Overrides the default `0.0.0.0` when set. | -| `HOSTNAME` | `127.0.0.1` | `scripts/dev/run-next-playwright.mjs` | Bind address used by the Playwright runner when launching Next.js. Defaults to `127.0.0.1` for hermetic tests. | +| `HOSTNAME` | `127.0.0.1` | `scripts/dev/run-next-playwright.mjs` | Bind address used by the Playwright runner when launching Next.js. Defaults to `127.0.0.1` for hermetic tests. **Do not use for `omniroute serve`** — use `OMNIROUTE_SERVER_HOST` instead (POSIX shells auto-set `HOSTNAME` to the machine name; `.env` cannot override it). | +| `OMNIROUTE_SERVER_HOST` | `0.0.0.0` | `bin/cli/commands/serve.mjs` | Bind address for `omniroute serve`. Avoids collision with the POSIX shell `HOSTNAME` variable (always set to the machine name by bash/zsh). Falls back to `0.0.0.0` when unset. (#6194) | ### Port Modes diff --git a/tests/unit/cli-serve-hostname.test.ts b/tests/unit/cli-serve-hostname.test.ts index 64573aa2c1..80eeb6ff9c 100644 --- a/tests/unit/cli-serve-hostname.test.ts +++ b/tests/unit/cli-serve-hostname.test.ts @@ -3,27 +3,70 @@ import assert from "node:assert/strict"; /** * Replicate the HOSTNAME resolution from bin/cli/commands/serve.mjs to verify - * that the spawned server honours a HOSTNAME provided via env/.env instead of - * always hardcoding "0.0.0.0" (#5134). Mirrors the in-file replication pattern - * used by cli-serve-port.test.ts (serve.mjs spawns processes, so the logic is - * tested in isolation rather than imported). + * the #6194 fix: POSIX shells auto-set HOSTNAME to the machine name, which + * collides with the bind address. The fix uses os.hostname() to detect the + * auto-set signature and ignores it, while preserving backward compatibility + * for explicit HOSTNAME values (e.g. Windows CMD/PowerShell users). + * + * Resolution order: + * 1. OMNIROUTE_SERVER_HOST (new dedicated var — always wins) + * 2. HOSTNAME if it does NOT match os.hostname() (legacy backward compat) + * 3. "0.0.0.0" (default) */ -function resolveHostname(envHostname: string | undefined): string { - return envHostname || "0.0.0.0"; +function resolveHostname( + envServerHost: string | undefined, + envHostname: string | undefined, + machineHostname: string +): string { + return envServerHost || (envHostname !== machineHostname ? envHostname : undefined) || "0.0.0.0"; } -test("serve hostname: honours HOSTNAME env var when set", () => { - assert.equal(resolveHostname("127.0.0.1"), "127.0.0.1"); +// --- OMNIROUTE_SERVER_HOST takes precedence --- + +test("serve hostname: OMNIROUTE_SERVER_HOST takes precedence over everything", () => { + assert.equal(resolveHostname("127.0.0.1", "myhostname", "myhostname"), "127.0.0.1"); }); -test("serve hostname: honours a specific bind interface", () => { - assert.equal(resolveHostname("192.168.0.15"), "192.168.0.15"); +test("serve hostname: OMNIROUTE_SERVER_HOST overrides an explicit HOSTNAME", () => { + assert.equal(resolveHostname("192.168.1.100", "10.0.0.1", "myhostname"), "192.168.1.100"); }); -test("serve hostname: falls back to 0.0.0.0 when HOSTNAME is unset", () => { - assert.equal(resolveHostname(undefined), "0.0.0.0"); +// --- POSIX shell auto-set detection (the #6194 bug) --- + +test("serve hostname: POSIX auto-set HOSTNAME (matches os.hostname()) is ignored (#6194)", () => { + // bash/zsh sets HOSTNAME=. When it matches os.hostname(), + // it's the auto-set signature — must be ignored. + assert.equal(resolveHostname(undefined, "myhostname", "myhostname"), "0.0.0.0"); }); -test("serve hostname: falls back to 0.0.0.0 when HOSTNAME is an empty string", () => { - assert.equal(resolveHostname(""), "0.0.0.0"); +// --- Backward compatibility for explicit HOSTNAME values --- + +test("serve hostname: explicit HOSTNAME (not matching os.hostname()) is preserved", () => { + // Windows CMD/PowerShell user who set HOSTNAME=192.168.1.50 in .env + // HOSTNAME != os.hostname() → treat as intentional user config + assert.equal(resolveHostname(undefined, "192.168.1.50", "myhostname"), "192.168.1.50"); +}); + +test("serve hostname: localhost as explicit HOSTNAME is preserved", () => { + assert.equal(resolveHostname(undefined, "localhost", "myhostname"), "localhost"); +}); + +// --- Default fallback --- + +test("serve hostname: falls back to 0.0.0.0 when both are unset", () => { + assert.equal(resolveHostname(undefined, undefined, "myhostname"), "0.0.0.0"); +}); + +test("serve hostname: falls back to 0.0.0.0 when both are empty strings", () => { + assert.equal(resolveHostname("", "", "myhostname"), "0.0.0.0"); +}); + +test("serve hostname: OMNIROUTE_SERVER_HOST empty string falls through to HOSTNAME check", () => { + // Empty string is falsy → falls through; HOSTNAME is auto-set → ignored → 0.0.0.0 + assert.equal(resolveHostname("", "myhostname", "myhostname"), "0.0.0.0"); +}); + +test("serve hostname: OMNIROUTE_SERVER_HOST empty string with explicit HOSTNAME", () => { + // Empty string is falsy → falls through; HOSTNAME != os.hostname() → used + assert.equal(resolveHostname("", "10.0.0.5", "myhostname"), "10.0.0.5"); });