Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion bin/cli/commands/serve.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ export async function runServe(opts = {}) {
PORT: String(dashboardPort),
DASHBOARD_PORT: String(dashboardPort),
API_PORT: String(apiPort),
HOSTNAME: "0.0.0.0",
HOSTNAME: process.env.HOSTNAME || "0.0.0.0",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

critical

⚠️ Critical Binding Issue with OS-Inherited HOSTNAME\n\nUsing process.env.HOSTNAME || \"0.0.0.0\" introduces a critical bug in environments where the OS or container runtime automatically exports the HOSTNAME environment variable (such as Docker, systemd, or various cloud VMs).\n\n#### The Problem:\n1. Docker Containers: Docker automatically sets HOSTNAME to the container ID (e.g., 3a4b5c6d7e8f). With this change, the server will attempt to bind to 3a4b5c6d7e8f (the container's internal IP) instead of 0.0.0.0. This completely breaks port forwarding, making the dashboard inaccessible from the host machine.\n2. Linux/macOS VPS: Many systems export the machine's hostname in the environment. If the hostname does not resolve to a local bindable interface, the server will fail to start with an EADDRNOTAVAIL error.\n\n#### The Solution:\nWe should only honor process.env.HOSTNAME if it is a user-configured bind address (like an IP or localhost) and not the system's default hostname. We can achieve this by comparing process.env.HOSTNAME against os.hostname().

Suggested change
HOSTNAME: process.env.HOSTNAME || "0.0.0.0",
HOSTNAME: (process.env.HOSTNAME && process.env.HOSTNAME !== (await import("node:os")).hostname()) ? process.env.HOSTNAME : "0.0.0.0",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

⚠️ Style Guide Violation: Missing Tests for Production Code\n\nAccording to the Repository Style Guide (Rule 9):\n> "Always include tests when changing production code (src/, open-sse/, electron/, bin/)."\n\nSince this PR modifies bin/cli/commands/serve.mjs (which is under bin/), you must include corresponding unit or integration tests (e.g., under tests/) to verify that the HOSTNAME environment variable is correctly respected and handled by the CLI command.

References
  1. Always include tests when changing production code (src/, open-sse/, electron/, bin/). (link)

NODE_ENV: "production",
NODE_OPTIONS: `--max-old-space-size=${memoryLimit}`,
};
Expand Down
29 changes: 29 additions & 0 deletions tests/unit/cli-serve-hostname.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
import test from "node:test";
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).
*/
function resolveHostname(envHostname: string | undefined): string {
return envHostname || "0.0.0.0";
}

test("serve hostname: honours HOSTNAME env var when set", () => {
assert.equal(resolveHostname("127.0.0.1"), "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: falls back to 0.0.0.0 when HOSTNAME is unset", () => {
assert.equal(resolveHostname(undefined), "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");
});