fix(machines): reach a PC's Ovis on the address it listens on (#1960)

changelog.d/remote-pc-nonloopback-host.md

@@ -0,0 +1,5 @@
+### Fixed
+- Adding a PC whose own Ovis listens on another address than 127.0.0.1 (its HOST set to, say, its
+  Tailscale address) connects instead of failing with "ECONNRESET": that Ovis now also answers the
+  app's SSH tunnel on the PC's 127.0.0.1. An older Ovis set up that way, or one whose HOST is a host
+  name, gets an error saying to update it or set HOST=127.0.0.1 there.

docs/remote-pcs.md

@@ -72,7 +72,7 @@ The current server keeps one status-only WebSocket per remote PC without attachi
 
 Legacy HTTP paths, `/ws` without a machine, and old stored preferences/notification links mean **local**. Machine mutations require `X-Herdr-Machine: 1`, the existing token gate and same-origin browser requests. The proxy passes a bridge credential read through SSH; browser cookies, bearer tokens, forwarded headers and remote cookies are not forwarded. Authentication, push devices, VAPID keys and app updates belong to the current connection server.
 
-Remote credentials are registered in `~/.config/herdr-web-ui/bridges/` on that PC (mode `0600`). A compatible bridge for the same socket is reused, including a normally started server from this version. Independently managed bridges are never stopped by remote setup. Older unregistered bridges can still hold an exclusive terminal attach; release their browser attachment or restart them with registration support. Automatic attaches and reconnects never use `--takeover`. With a bridge that advertises support,
+Remote credentials are registered in `~/.config/herdr-web-ui/bridges/` on that PC (mode `0600`). A compatible bridge for the same socket is reused, including a normally started server from this version. A server started with `HOST` set to an IP address other than `127.0.0.1` or a wildcard also opens a relay on `127.0.0.1` and registers that port, since the forward only ever reaches the PC's loopback. A host name in `HOST` gets no relay, since it could resolve to another machine by the next connection; adding that PC says to set `HOST=127.0.0.1`. Independently managed bridges are never stopped by remote setup. Older unregistered bridges can still hold an exclusive terminal attach; release their browser attachment or restart them with registration support. Automatic attaches and reconnects never use `--takeover`. With a bridge that advertises support,
 **Open here** on a pane's waiting notice explicitly takes its terminal. The displaced web bridge
 waits; a standalone `herdr terminal attach` exits. The herdr TUI stays connected (verified on
 herdr 0.9.3). Older bridge bundles keep waiting until updated, and observe connections cannot take a pane.

server/bridge.test.ts

@@ -2,7 +2,8 @@ import { afterEach, beforeEach, describe, expect, it, spyOn } from "bun:test";
 import { existsSync, mkdirSync, mkdtempSync, readdirSync, readFileSync, rmSync, writeFileSync } from "node:fs";
 import { tmpdir } from "node:os";
 import { join } from "node:path";
-import { descriptorLive, registerBridge, type BridgeDescriptor } from "./bridge.ts";
+import type { Socket } from "bun";
+import { descriptorLive, loopbackRelay, registerBridge, RELAY_CLIENT_AHEAD_BYTES, relayTarget, type BridgeDescriptor } from "./bridge.ts";
 import { guardLibrary, guardPath, OwnerGuardReleaseFailed } from "./owner-guard.ts";
 import { readProcessBirth } from "./process-owner.ts";
 
@@ -190,3 +191,148 @@ describe("bridge registration", () => {
     expect(message).not.toContain("new-token");
   });
 });
+
+// #1960: an Ovis with HOST set to another address registers this relay, since the app's tunnel reaches only the PC's loopback
+describe("loopback relay", () => {
+  const stops: (() => void)[] = [];
+  afterEach(() => { for (const stop of stops.splice(0)) stop(); });
+  function relayTo(port: number): number {
+    const relay = loopbackRelay("127.0.0.1", port);
+    stops.push(() => relay.stop());
+    return relay.port;
+  }
+
+  it("passes requests, a large answer and a WebSocket through to the server", async () => {
+    const big = "x".repeat(8 * 1024 * 1024);
+    const server = Bun.serve({
+      port: 0, hostname: "127.0.0.1",
+      fetch(request, srv) {
+        if (new URL(request.url).pathname === "/ws") return srv.upgrade(request) ? undefined : new Response("no", { status: 400 });
+        return new Response(new URL(request.url).pathname === "/big" ? big : `${request.method} ${request.headers.get("authorization")}`);
+      },
+      websocket: { message(ws, message) { ws.send(`echo ${message}`); } },
+    });
+    stops.push(() => server.stop(true));
+    const port = relayTo(server.port!);
+    expect(port).not.toBe(server.port);
+    expect(await (await fetch(`http://127.0.0.1:${port}/api/bridge`, { headers: { authorization: "Bearer t" } })).text()).toBe("GET Bearer t");
+    expect((await (await fetch(`http://127.0.0.1:${port}/big`)).text()).length).toBe(big.length);
+    const ws = new WebSocket(`ws://127.0.0.1:${port}/ws`);
+    const echoed = new Promise<string>((resolve, reject) => { ws.onmessage = (event) => resolve(String(event.data)); ws.onerror = () => reject(new Error("WebSocket failed")); });
+    ws.onopen = () => ws.send("hi");
+    expect(await echoed).toBe("echo hi");
+    ws.close();
+  });
+
+  it("makes the server wait for a client that stops reading, then delivers every byte", async () => {
+    const size = 64 * 1024 * 1024;
+    const piece = 16 * 1024;
+    const blocked = Promise.withResolvers<"blocked">();
+    const sent = Promise.withResolvers<"sent">();
+    // one piece per event-loop turn, so the relay gets to read between them; a write that takes
+    // less than a piece is the server feeling the client's backpressure, and its drain goes on
+    // every byte says where it belongs, so a lost, repeated or reordered piece shows
+    const payload = new Uint8Array(size);
+    for (let i = 0; i < size; i++) payload[i] = (i ^ (i >>> 13)) & 0xff;
+    type Writer = { data: { at: number }; write(chunk: Uint8Array): number; end(): void };
+    const pump = (socket: Writer): void => {
+      if (socket.data.at === size) { sent.resolve("sent"); socket.end(); return; }
+      const want = Math.min(piece, size - socket.data.at);
+      const written = Math.max(socket.write(payload.subarray(socket.data.at, socket.data.at + want)), 0);
+      socket.data.at += written;
+      if (written < want) { blocked.resolve("blocked"); return; }
+      setImmediate(() => pump(socket));
+    };
+    const server = Bun.listen<{ at: number }>({ hostname: "127.0.0.1", port: 0, socket: {
+      open(socket) { socket.data = { at: 0 }; pump(socket); },
+      drain: pump,
+      data() {},
+    } });
+    stops.push(() => server.stop(true));
+    const received = new Uint8Array(size);
+    let filled = 0;
+    const done = Promise.withResolvers<void>();
+    const client = await Bun.connect({ hostname: "127.0.0.1", port: relayTo(server.port), socket: {
+      open(socket) { socket.pause(); },
+      data(_socket, data) { received.set(data.subarray(0, size - filled), filled); filled += data.byteLength; },
+      close() { done.resolve(); },
+    } });
+    // a relay that buffered everything would let the server finish while nothing is read
+    expect(await Promise.race([blocked.promise, sent.promise])).toBe("blocked");
+    client.resume();
+    await done.promise;
+    expect(filled).toBe(size);
+    expect(Buffer.compare(received, payload)).toBe(0);
+  });
+
+  /** a client sending `total` bytes through the relay to a server that reads nothing */
+  async function stuckUpstream(total: number): Promise<{ client: Socket; relay: { stop(): void }; written: Promise<void>; clientClosed: Promise<void>; upstreamClosed: Promise<void> }> {
+    const upstreamClosed = Promise.withResolvers<void>();
+    const server = Bun.listen({ hostname: "127.0.0.1", port: 0, socket: {
+      open(socket) { socket.pause(); },
+      data() {},
+      close() { upstreamClosed.resolve(); },
+    } });
+    stops.push(() => server.stop(true));
+    const relay = loopbackRelay("127.0.0.1", server.port);
+    stops.push(() => relay.stop());
+    const written = Promise.withResolvers<void>();
+    const clientClosed = Promise.withResolvers<void>();
+    const chunk = new Uint8Array(1024 * 1024).fill(3);
+    let sent = 0;
+    const push = (socket: Socket) => {
+      while (sent < total) {
+        const want = Math.min(chunk.byteLength, total - sent);
+        const accepted = socket.write(chunk.subarray(0, want));
+        if (accepted <= 0) return;
+        sent += accepted;
+        if (accepted < want) return;
+      }
+      written.resolve();
+    };
+    const client = await Bun.connect({ hostname: "127.0.0.1", port: relay.port, socket: { open: push, drain: push, data() {}, close() { clientClosed.resolve(); } } });
+    return { client, relay, written: written.promise, clientClosed: clientClosed.promise, upstreamClosed: upstreamClosed.promise };
+  }
+
+  it("lets go of a server that reads nothing once its client leaves", async () => {
+    const { client, written, upstreamClosed } = await stuckUpstream(16 * 1024 * 1024);
+    await written;
+    client.terminate();
+    await upstreamClosed;
+  });
+
+  it("lets go of a server that reads nothing when it stops", async () => {
+    const { relay, written, upstreamClosed } = await stuckUpstream(16 * 1024 * 1024);
+    await written;
+    relay.stop();
+    await upstreamClosed;
+  });
+
+  it("cuts off a client that gets too far ahead of a server that reads nothing", async () => {
+    const { clientClosed, upstreamClosed } = await stuckUpstream(RELAY_CLIENT_AHEAD_BYTES + 16 * 1024 * 1024);
+    await Promise.all([clientClosed, upstreamClosed]);
+  });
+
+  // macOS gives its loopback interface 127.0.0.1 alone
+  it.skipIf(process.platform === "darwin")("reaches a server on another loopback address", async () => {
+    const server = Bun.serve({ port: 0, hostname: "127.0.0.2", fetch: () => new Response("two") });
+    stops.push(() => server.stop(true));
+    expect(relayTarget("127.0.0.2")).toBe("127.0.0.2");
+    const relay = loopbackRelay("127.0.0.2", server.port!);
+    stops.push(() => relay.stop());
+    expect(await (await fetch(`http://127.0.0.1:${relay.port}/`)).text()).toBe("two");
+  });
+
+  it("closes a connection when nothing listens behind it", async () => {
+    const gone = Bun.listen({ hostname: "127.0.0.1", port: 0, socket: { data() {} } });
+    const closed = gone.port;
+    gone.stop(true);
+    await expect(fetch(`http://127.0.0.1:${relayTo(closed)}/`, { signal: AbortSignal.timeout(5000) })).rejects.toThrow();
+  });
+
+  it("relays an IP address that does not answer on 127.0.0.1, and never a host name", () => {
+    expect(["127.0.0.2", "100.64.0.7", "::1", "[FD7A:115C:A1E0::7]"].map(relayTarget)).toEqual(["127.0.0.2", "100.64.0.7", "::1", "fd7a:115c:a1e0::7"]);
+    // 127.0.0.1 and a wildcard need none; a host name may resolve elsewhere by the next connection
+    expect(["127.0.0.1", "0.0.0.0", "::", "[::]", "localhost", "pc.example", "pc.tail1234.ts.net"].map(relayTarget)).toEqual([null, null, null, null, null, null, null]);
+  });
+});

server/bridge.ts

@@ -1,14 +1,20 @@
 import { createHash, randomBytes } from "node:crypto";
 import { chmodSync, mkdirSync, readFileSync, realpathSync, renameSync, statSync, unlinkSync, writeFileSync } from "node:fs";
+import { isIP } from "node:net";
 import { homedir } from "node:os";
 import { dirname, join, resolve } from "node:path";
 import { BRIDGE_PROTOCOL, REMOTE_BUNDLE_VERSION, type BridgeIdentity } from "../shared/machines.ts";
 import { HerdrError, herdrSocketPath, ping } from "./herdr/client.ts";
 import { OwnerGuardUnavailable, withOwnerGuard } from "./owner-guard.ts";
 import { newOwnerRecord, ownerGone, probeProcess, type ProcessBirth } from "./process-owner.ts";
+import type { Socket } from "bun";
+import { MAX_IMAGE_BODY_BYTES } from "./paste.ts";
 import { attachableIdentity, sidecarAvailable } from "./pty/sidecar.ts";
 
-/** `generation` and `birth` identify the registering process beyond its PID; older bridges wrote neither. */
+/**
+ * `generation` and `birth` identify the registering process beyond its PID; older bridges wrote neither.
+ * `port` is on 127.0.0.1, the only address the app's SSH tunnel goes to (`loopbackRelay`).
+ */
 export interface BridgeDescriptor { managed_remote?: boolean; port: number; token: string; pid: number; generation?: string; birth?: ProcessBirth; socket_path: string; bridge_protocol: number; bundle_version: string }
 export function bridgeRegistry(): string { return join(homedir(), ".config/herdr-web-ui/bridges"); }
 export function socketPath(): string {
@@ -107,3 +113,95 @@ export function registerBridge(port: number, token = randomBytes(32).toString("h
     },
   };
 }
+
+/**
+ * The address a loopback relay passes connections to for a server bound to `hostname`, or null when
+ * it needs none (127.0.0.1 or a wildcard already answers on 127.0.0.1; 127.0.0.2 does not) or gets
+ * none: a host name is not relayed, since by the next connection it may resolve to another machine
+ * and take the bridge token there. Such a server gets the app's message to set HOST=127.0.0.1.
+ */
+export function relayTarget(hostname: string): string | null {
+  const name = hostname.trim().toLowerCase().replace(/^\[(.*)\]$/, "$1");
+  if (name === "127.0.0.1" || name === "0.0.0.0" || name === "::" || isIP(name) === 0) return null;
+  return name;
+}
+
+/**
+ * How far a client may get ahead of a server that is not reading before the relay cuts both off: the
+ * largest request body the server reads (a picture at the attachment limit), twice over.
+ */
+export const RELAY_CLIENT_AHEAD_BYTES = 2 * MAX_IMAGE_BODY_BYTES;
+/** `queue`: what this socket has not taken yet, `queued` bytes; `ending`: the other side closed, so this one ends once its queue is sent */
+interface Relayed { peer?: Socket<Relayed>; queue: Uint8Array[]; queued: number; closed?: boolean; ending?: boolean }
+/** writes `chunk` after what the socket has not taken yet (`drain` sends the rest); the bytes it holds now */
+function send(socket: Socket<Relayed>, chunk: Uint8Array): number {
+  if (!socket.data.queue.length) {
+    const written = Math.max(socket.write(chunk), 0);
+    if (written === chunk.byteLength) return 0;
+    chunk = chunk.subarray(written);
+  }
+  socket.data.queue.push(chunk);
+  socket.data.queued += chunk.byteLength;
+  return socket.data.queued;
+}
+function drain(socket: Socket<Relayed>): void {
+  while (socket.data.queue.length) {
+    const chunk = socket.data.queue[0]!;
+    const written = Math.max(socket.write(chunk), 0);
+    socket.data.queued -= written;
+    if (written < chunk.byteLength) { socket.data.queue[0] = chunk.subarray(written); return; }
+    socket.data.queue.shift();
+  }
+  if (socket.data.ending) socket.end();
+  else socket.data.peer?.resume();
+}
+/** ends `socket` after what it still has to send */
+function endAfterQueue(socket: Socket<Relayed> | undefined): void {
+  if (!socket) return;
+  socket.data.ending = true;
+  if (!socket.data.queue.length) socket.end();
+}
+
+/**
+ * A port on 127.0.0.1 that passes each connection on to `target:port` (#1960), `target` an IP address (`relayTarget`). An Ovis whose HOST
+ * is another address does not answer on the PC's loopback, where the app's SSH tunnel always goes:
+ * it registers this port as its bridge instead. The tunnel then never reaches past the PC's loopback,
+ * whatever a descriptor says, and a request through it still arrives from that address, not loopback.
+ *
+ * A client slow to read pauses the server's side, so the server feels it as its own backpressure (its
+ * output-stall checks read it) instead of the relay holding it all. The client's side is never paused:
+ * a paused socket would not see its client leave. A client more than RELAY_CLIENT_AHEAD_BYTES ahead of
+ * a server that is not reading is cut off instead.
+ */
+export function loopbackRelay(target: string, port: number): { port: number; stop(): void } {
+  const listener = Bun.listen<Relayed>({ hostname: "127.0.0.1", port: 0, socket: {
+    open(client) {
+      client.data = { queue: [], queued: 0 };
+      // read nothing from the client until the server's side is there to take it
+      client.pause();
+      void Bun.connect<Relayed>({ hostname: target, port, data: { queue: [], queued: 0 }, socket: {
+        open(peer) {
+          if (client.data.closed) { peer.end(); return; }
+          peer.data.peer = client;
+          client.data.peer = peer;
+          client.resume();
+        },
+        data(peer, chunk) { if (send(client, chunk)) peer.pause(); },
+        drain,
+        close() { endAfterQueue(client); },
+        error() { endAfterQueue(client); },
+      } }).catch(() => client.end());
+    },
+    data(client, chunk) {
+      const peer = client.data.peer;
+      if (peer && send(peer, chunk) > RELAY_CLIENT_AHEAD_BYTES) { client.terminate(); peer.terminate(); }
+    },
+    drain,
+    // nothing a departed client sent still matters, and a server that reads nothing would hold the
+    // queue forever: its side goes at once (a server's own close still lets the client read the rest)
+    close(client) { client.data.closed = true; client.data.peer?.terminate(); },
+    error(client) { client.data.closed = true; client.data.peer?.terminate(); },
+  } });
+  // stopping closes every client, and each client's close takes its server's side with it
+  return { port: listener.port, stop: () => listener.stop(true) };
+}

server/index.ts

@@ -50,7 +50,7 @@ import { UsageLoginService } from "./usage-login.ts";
 import { handleVoiceRequest, VoiceService } from "./voice.ts";
 
 import { BRIDGE_PROTOCOL } from "../shared/machines.ts";
-import { bridgeIdentity, registerBridge } from "./bridge.ts";
+import { bridgeIdentity, loopbackRelay, registerBridge, relayTarget } from "./bridge.ts";
 import { handleMachineRequest, nonCanonicalPath, slowMachinePath } from "./machine-api.ts";
 import { handleSessionHistoryRequest, handleSessionResumeRequest, SESSION_HISTORY_PATH, SESSION_RESUME_PATH } from "./session-history-routes.ts";
 import { HerdrSidebarConfig, herdrConfigPath } from "./herdr-sidebar.ts";
@@ -608,7 +608,10 @@ export function createServer(
     settlePortalStartup({ ok: true });
   }
 
-  const registration = options.registerBridge ? registerBridge(server.port ?? 0, bridgeToken) : null;
+  // the app's SSH tunnel reaches a bridge on the PC's 127.0.0.1 only (#1960)
+  const target = options.registerBridge ? relayTarget(hostname) : null;
+  const relay = target ? loopbackRelay(target, server.port ?? 0) : null;
+  const registration = options.registerBridge ? registerBridge(relay?.port ?? server.port ?? 0, bridgeToken) : null;
 
   core.start();
 
@@ -628,6 +631,7 @@ export function createServer(
         openCodeState.clear();
         void options.portal?.shutdown();
         registration?.close();
+        relay?.stop();
         closeCodexDaemonSources();
       });
       void shellAgents.stop();