Skip to content

Commit 6b5daee

Browse files
lidge-junclaude
andcommitted
fix(deploy): close windows-deploy-stability audit — restart EINVAL, systemd SSH, localhost bind
Audit (5 parallel reviewers) of devlog 260702_windows-deploy-stability confirmed all prior-loop fixes landed except two, now fixed: - update-job restart spawned ocx.cmd shell-less → EINVAL on Windows bun/source GUI restart (CVE-2024-27980 class). Restart now uses process.execPath + package launcher (real exe, no shell). - F9: isSystemd() hard-failed on `systemctl --user show-environment` in no-DBUS SSH sessions, wrongly refusing first-time install. Add userRuntimeDir()/ensureUserBusEnv(): set XDG_RUNTIME_DIR to /run/user/<uid> and fall back to its existence as the systemd-present signal. - Bind canonicalizes explicit hostname "localhost" → 127.0.0.1 (F4 symmetry); wildcards untouched. Guarded by tests/windows-deploy-close-regressions.test.ts. tsc 0, suite 1375 pass/0 fail. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 69c56e3 commit 6b5daee

4 files changed

Lines changed: 87 additions & 8 deletions

File tree

src/server.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2102,9 +2102,15 @@ export function startServer(port?: number) {
21022102
const listenPort = port ?? config.port ?? 10100;
21032103
setCorsOrigin(listenPort);
21042104

2105+
// Canonicalize an explicit "localhost" bind to IPv4 so it matches the injected base_url (which
2106+
// resolves localhost→127.0.0.1): on Windows `localhost` resolves ::1-first, but the injected URL
2107+
// is 127.0.0.1, so binding literal "localhost" would reintroduce the F4 refusal. Wildcards
2108+
// (0.0.0.0/::) and specific hosts are left untouched so intentional exposure is preserved.
2109+
const bindHost = /^localhost$/i.test(config.hostname ?? "") ? "127.0.0.1" : (config.hostname ?? "127.0.0.1");
2110+
21052111
const server: Server<WsData> = Bun.serve<WsData>({
21062112
port: listenPort,
2107-
hostname: config.hostname ?? "127.0.0.1",
2113+
hostname: bindHost,
21082114
idleTimeout: 255,
21092115
async fetch(req, requestServer): Promise<Response> {
21102116
const url = new URL(req.url);

src/service.ts

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -493,12 +493,41 @@ WantedBy=default.target
493493
`;
494494
}
495495

496+
/** The per-user runtime dir systemd creates (holds the user-bus socket), or null. */
497+
function userRuntimeDir(): string | null {
498+
const fromEnv = process.env.XDG_RUNTIME_DIR;
499+
if (fromEnv && existsSync(fromEnv)) return fromEnv;
500+
if (typeof process.getuid === "function") {
501+
const candidate = `/run/user/${process.getuid()}`;
502+
if (existsSync(candidate)) return candidate;
503+
}
504+
return null;
505+
}
506+
507+
/**
508+
* SSH sessions frequently start without `XDG_RUNTIME_DIR`/`DBUS_SESSION_BUS_ADDRESS`, so
509+
* `systemctl --user` can't find the user bus even when systemd is running. Point `XDG_RUNTIME_DIR`
510+
* at the per-user runtime dir when it exists so the `--user` probe and install commands reach the
511+
* bus. No-op when already set or when no runtime dir exists (e.g. genuinely non-systemd hosts).
512+
*/
513+
function ensureUserBusEnv(): void {
514+
if (process.env.XDG_RUNTIME_DIR) return;
515+
const dir = userRuntimeDir();
516+
if (dir) process.env.XDG_RUNTIME_DIR = dir;
517+
}
518+
496519
function isSystemd(): boolean {
497520
try { execSync("systemctl --version", { stdio: "pipe" }); } catch { return false; }
498-
try { execSync("systemctl --user show-environment", { stdio: "pipe" }); return true; } catch { return false; }
521+
ensureUserBusEnv();
522+
// Prefer the user-bus probe; but an SSH session without a user D-Bus fails it even when systemd
523+
// is present (F9). Fall back to the per-user runtime dir existing — a strong signal the user
524+
// systemd instance is available — so a first-time `ocx service install` isn't wrongly refused.
525+
try { execSync("systemctl --user show-environment", { stdio: "pipe" }); return true; } catch { /* no user bus in this session */ }
526+
return userRuntimeDir() !== null;
499527
}
500528

501529
function installSystemd(): void {
530+
ensureUserBusEnv(); // reach the user bus over a bare SSH session (F9)
502531
const dir = unitDir();
503532
if (!existsSync(dir)) mkdirSync(dir, { recursive: true });
504533
if (!existsSync(getConfigDir())) mkdirSync(getConfigDir(), { recursive: true });

src/update-job.ts

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -78,10 +78,6 @@ function nodeBin(): string {
7878
return process.platform === "win32" ? "node.exe" : "node";
7979
}
8080

81-
function ocxBin(): string {
82-
return process.platform === "win32" ? "ocx.cmd" : "ocx";
83-
}
84-
8581
function packageLauncherPath(): string {
8682
return join(dirname(fileURLToPath(import.meta.url)), "..", "bin", "ocx.mjs");
8783
}
@@ -163,8 +159,11 @@ export function restartCommand(
163159
const args = serviceInstalled ? [launcher, "service", "install"] : [launcher, "start"];
164160
return { mode, bin, args, display: formatCommand(bin, args) };
165161
}
166-
const bin = ocxBin();
167-
const args = serviceInstalled ? ["service", "install"] : ["start"];
162+
// bun/source installs: restart via the current runtime executable + package launcher (both real
163+
// .exe files), NOT the `ocx.cmd` shim. Spawning a `.cmd` shell-less throws EINVAL on Windows
164+
// Node/Bun ≥18.20/20.12 (CVE-2024-27980 hardening) — the same class the npm path (nodeBin) avoids.
165+
const bin = process.execPath;
166+
const args = serviceInstalled ? [launcher, "service", "install"] : [launcher, "start"];
168167
return { mode, bin, args, display: formatCommand(bin, args) };
169168
}
170169

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
import { describe, expect, test } from "bun:test";
2+
import { readFileSync } from "node:fs";
3+
import { join } from "node:path";
4+
5+
// Source-contract regressions for the final fixes that let devlog
6+
// 260702_windows-deploy-stability close: the ocx.cmd shell-less restart (A), F9 systemd no-DBUS
7+
// SSH detection (E), and the F4 explicit-localhost bind symmetry (D). These files run top-level or
8+
// platform-gated logic, so guard the invariants at the source level (repo convention — see
9+
// ocx-launcher-source.test.ts / service.test.ts).
10+
const read = (rel: string) => readFileSync(join(import.meta.dir, "..", rel), "utf8");
11+
12+
describe("update-job restart avoids the shell-less .cmd EINVAL (Windows, bun/source)", () => {
13+
const src = read("src/update-job.ts");
14+
test("no ocx.cmd shim is spawned for restart", () => {
15+
expect(src).not.toContain('"ocx.cmd"');
16+
expect(src).not.toMatch(/function ocxBin/);
17+
});
18+
test("bun/source restart uses the runtime executable + launcher (a real .exe, no shell)", () => {
19+
// restartCommand's non-npm branch resolves to process.execPath + the package launcher.
20+
expect(src).toMatch(/const bin = process\.execPath;\s*\n\s*const args = serviceInstalled \? \[launcher, "service", "install"\] : \[launcher, "start"\];/);
21+
});
22+
});
23+
24+
describe("systemd detection tolerates a no-DBUS SSH session (F9)", () => {
25+
const src = read("src/service.ts");
26+
test("isSystemd falls back to the per-user runtime dir when the user-bus probe fails", () => {
27+
expect(src).toContain("function userRuntimeDir()");
28+
expect(src).toContain("function ensureUserBusEnv()");
29+
// The version probe passing + a runtime dir existing is enough — not a hard fail on the --user probe.
30+
expect(src).toMatch(/catch \{ \/\* no user bus in this session \*\/ \}\s*\n\s*return userRuntimeDir\(\) !== null;/);
31+
});
32+
test("install ensures the user-bus env before touching systemctl --user", () => {
33+
expect(src).toMatch(/function installSystemd\(\): void \{\s*\n\s*ensureUserBusEnv\(\);/);
34+
});
35+
});
36+
37+
describe("server bind canonicalizes explicit localhost but preserves wildcards (F4 symmetry)", () => {
38+
const src = read("src/server.ts");
39+
test("literal localhost binds to 127.0.0.1; 0.0.0.0/:: exposure is untouched", () => {
40+
expect(src).toContain('/^localhost$/i.test(config.hostname ?? "") ? "127.0.0.1"');
41+
expect(src).toContain("hostname: bindHost,");
42+
// Must not blanket-rewrite the bind host (that would break intentional 0.0.0.0 exposure).
43+
expect(src).not.toContain('hostname: "127.0.0.1",');
44+
});
45+
});

0 commit comments

Comments
 (0)