Check WS Origin against the server's own origins, not the Host header

DESIGN.md

@@ -874,3 +874,15 @@ The Linux app was not affected by the image bug: its Chromium is confined with
 `--host-resolver-rules` and an unreachable `--proxy-server`, neither of which applies to `blob:`
 URLs, which never reach the network stack. There, TIFF and HEIC (which Chromium cannot decode) now
 open as the hex dump under a notice.
+
+## Security review, October 2026
+
+An adversarial review (three parallel reviewers plus RED-test validation of the top
+findings) re-walked the server surface. One finding was validated and fixed; several
+were refuted or accepted as trade-offs.
+
+| Finding | Fix |
+| --- | --- |
+| The WebSocket Origin check compared `Origin` against the request's own `Host` header, so under a non-loopback `--host` (where the DNS-rebinding `hostOk` allowlist is skipped) a page could connect with a self-consistent attacker Host+Origin pair and get `101 Switching Protocols`. RED: `test/http.wsorigin.security.test.ts`. | Origin is now checked against the server's own origins — loopback names plus every local interface address (`os.networkInterfaces()`), never anything client-controlled. Deliberate consequence: the origin host must be a literal loopback or interface address; reaching the server via a DNS/mDNS name is rejected, because resolving a client-supplied hostname back to a local address would reopen the rebinding hole. |
+| (Review) `alias.*`/`pager.*` in a hostile repo config were claimed to run commands. | Refuted by test: git does not expand an alias that shadows a builtin subcommand (`test/git.alias.security.test.ts` pins this, marker file stays empty). Kept as a regression net. |
+| (Review) `HARDEN` overrides travel as `-c key=value` despite the GIT_CONFIG_KEY_n rule; token in the WS query string; missing-Origin bypass; regex CPU burn in the search worker; pty replay resync edge cases. | Not exploitable as written (values self-authored; query token is the documented WS mechanism; local DoS only). No change; recorded here for the next pass. |

src/server/http.ts

@@ -1,6 +1,7 @@
 import type { Server, ServerWebSocket } from "bun";
 import { randomBytes, timingSafeEqual } from "node:crypto";
 import path from "node:path";
+import os from "node:os";
 import { realpath, stat } from "node:fs/promises";
 import { listDir, readForView, resolveSafe, imageMime, toRel, looksBinary, openRegular, MAX_TEXT_BYTES } from "./files.ts";
 import { FileIndex } from "./index.ts";
@@ -66,6 +67,23 @@
   let port = 0;
   const loopback = ["127.0.0.1", "localhost", "::1"].includes(opts.host);
 
+  /**
+   * Every origin a legitimate browser page can have when talking to this server:
+   * loopback names plus every local interface address (a non-loopback --host can be
+   * reached under any of them). Never derived from the request's Host header, which
+   * the client controls — matching Origin against it made the cross-site WebSocket
+   * guard a no-op under DNS rebinding.
+   */
+  const selfOrigins = (): Set<string> => {
+    const set = new Set<string>([`http://localhost:${port}`, `http://127.0.0.1:${port}`, `http://[::1]:${port}`]);
+    if (!loopback) {
+      for (const addrs of Object.values(os.networkInterfaces())) {
+        for (const a of addrs ?? []) set.add(`http://${a.address.includes(":") ? `[${a.address}]` : a.address}:${port}`);
+      }
+    }
+    return set;
+  };
+
   const broadcast = (msg: unknown) => {
     const s = JSON.stringify(msg);
     for (const ws of events) ws.send(s);
@@ -262,8 +280,9 @@
 
       if (p.startsWith("/ws/")) {
         // Cross-site WebSocket hijacking guard: browsers always send Origin; it must be us.
+        // Compared against this server's own origins, never the client-controlled Host.
         const o = req.headers.get("origin");
-        if (o && o !== `http://${req.headers.get("host")}`) return fail(403, "bad origin");
+        if (o && !selfOrigins().has(o)) return fail(403, "bad origin");
         if (p === "/ws/events") {
           return srv.upgrade(req, { data: { kind: "events" } }) ? undefined : fail(400, "upgrade failed");
         }
@@ -380,7 +399,7 @@
               if (bytesRead === 0) {
                 await fh.close();
                 // The file shrank after fstat: fail the body rather than send fewer bytes than promised.
                 return pos < stop ? ctl.error(new Error("file changed while reading")) : ctl.close();
               }
               pos += bytesRead;
               ctl.enqueue(chunk.subarray(0, bytesRead));

test/git.alias.security.test.ts

@@ -0,0 +1,60 @@
+import { afterAll, beforeAll, expect, test } from "bun:test";
+import { mkdtemp, rm, writeFile, mkdir, readFile, chmod, realpath } from "node:fs/promises";
+import { existsSync } from "node:fs";
+import { tmpdir } from "node:os";
+import path from "node:path";
+import { Git } from "../src/server/git.ts";
+
+// RED tests for the 2026-10 security review: a repository whose .git/config defines
+// alias.* entries must not be able to make codeviewer's git calls run programs.
+// git.ts's neutralize() only blanks filter./diff./merge. drivers (git.ts:191);
+// alias.* is a separate config family that expands a shell command when the invoked
+// subcommand name matches (e.g. `git status`, `git log` as called at git.ts:242-244).
+let dir = "";
+let repo = "";
+let marker = "";
+
+const sh = (cmd: string[], cwd = repo) => {
+  const r = Bun.spawnSync(["git", ...cmd], { cwd, stderr: "pipe" });
+  if (r.exitCode !== 0) throw new Error(`git ${cmd.join(" ")}: ${r.stderr}`);
+  return r.stdout.toString();
+};
+
+beforeAll(async () => {
+  dir = await realpath(await mkdtemp(path.join(tmpdir(), "cv-gitalias-")));
+  repo = path.join(dir, "repo");
+  marker = path.join(dir, "pwned.log");
+  await mkdir(repo);
+  const evil = path.join(dir, "evil.sh");
+  await writeFile(evil, `#!/bin/sh\necho "ran $*" >> '${marker}'\n`);
+  await chmod(evil, 0o755);
+  sh(["init", "-q", "-b", "main"]);
+  sh(["config", "user.email", "a@b"]);
+  sh(["config", "user.name", "a"]);
+  await writeFile(path.join(repo, "a.txt"), "one\ntwo\n");
+  sh(["add", "a.txt"]);
+  sh(["commit", "-qm", "init"]);
+  // Attack: aliases shadowing the exact subcommands codeviewer invokes.
+  sh(["config", "alias.status", `!${evil} status`]);
+  sh(["config", "alias.log", `!${evil} log`]);
+});
+
+afterAll(async () => {
+  await rm(dir, { recursive: true, force: true });
+});
+
+test("a malicious repository cannot define alias.* that runs programs on status()", async () => {
+  const g = (await Git.open(repo))!;
+  expect(g).not.toBeNull();
+  const s = await g.status();
+  // The status call itself must have worked (aliases neutralized, builtin ran).
+  expect(s.head?.subject).toBe("init");
+  expect(existsSync(marker) ? await readFile(marker, "utf8") : "").toBe("");
+});
+
+test("a malicious repository cannot define alias.* that runs programs on history()", async () => {
+  const g = (await Git.open(repo))!;
+  const h = await g.history("a.txt");
+  expect(h!.length).toBeGreaterThan(0);
+  expect(existsSync(marker) ? await readFile(marker, "utf8") : "").toBe("");
+});

test/http.wsorigin.security.test.ts

@@ -0,0 +1,68 @@
+import { afterAll, beforeAll, expect, test } from "bun:test";
+import { mkdtemp, rm, writeFile, mkdir, realpath } from "node:fs/promises";
+import { tmpdir } from "node:os";
+import path from "node:path";
+import net from "node:net";
+import { startServer } from "../src/server/http.ts";
+
+// RED test for the 2026-10 security review: when the server binds a non-loopback
+// host, hostOk() (http.ts:115-119) short-circuits to true, so the only DNS-rebinding
+// / cross-site-WebSocket guard left is the Origin check at http.ts:266 — which
+// compares Origin against the request's own Host header. An attacker page on
+// attacker.example that connects to <lan-ip>:<port> with Host: attacker.example:port
+// sends Origin http://attacker.example:port, the two match, and the upgrade is
+// accepted. The Origin check must compare against the server's own origin
+// (scheme + bound host + port), not the client-controlled Host.
+const token = "S".repeat(43);
+
+let dir = "";
+let app: ReturnType<typeof startServer>;
+let port = 0;
+
+/** Raw HTTP/1.1 upgrade request; returns the response status line. */
+const upgrade = (host: string, origin: string | null) =>
+  new Promise<string>((resolve, reject) => {
+    const sock = net.connect(port, "127.0.0.1", () => {
+      const lines = [
+        "GET /ws/events?t=" + token + " HTTP/1.1",
+        `Host: ${host}`,
+        "Upgrade: websocket",
+        "Connection: Upgrade",
+        "Sec-WebSocket-Key: dGhlIHNhbXBsZSBub25jZQ==",
+        "Sec-WebSocket-Version: 13",
+      ];
+      if (origin) lines.push(`Origin: ${origin}`);
+      sock.write(lines.join("\r\n") + "\r\n\r\n");
+    });
+    sock.once("data", (d) => {
+      resolve(d.toString("latin1").split("\r\n")[0]!);
+      sock.destroy();
+    });
+    sock.once("error", reject);
+    setTimeout(() => { sock.destroy(); reject(new Error("timeout")); }, 5000).unref();
+  });
+
+beforeAll(async () => {
+  process.env.CODEVIEWER_NO_UPDATE_CHECK = "1";
+  dir = await realpath(await mkdtemp(path.join(tmpdir(), "cv-wsorigin-")));
+  await mkdir(path.join(dir, "sub"));
+  await writeFile(path.join(dir, "a.txt"), "line1\n");
+  app = startServer({ root: dir, port: 0, host: "0.0.0.0", shell: "/bin/sh", token });
+  port = app.server.port!;
+});
+
+afterAll(async () => {
+  app.stop();
+  await rm(dir, { recursive: true, force: true });
+});
+
+test("a WS upgrade whose Origin matches a spoofed Host header is rejected", async () => {
+  // DNS-rebinding shape: attacker-controlled hostname in both Host and Origin.
+  const status = await upgrade("attacker.example:1", "http://attacker.example:1");
+  expect(status).toContain("403");
+});
+
+test("a WS upgrade with the server's real origin is still accepted", async () => {
+  const status = await upgrade(`127.0.0.1:${port}`, `http://127.0.0.1:${port}`);
+  expect(status).toContain("101");
+});