From 67692bb4423afd113b78b9d6f303c0592e3189ac Mon Sep 17 00:00:00 2001 From: lntvan166 Date: Fri, 21 Aug 2026 16:03:26 +0700 Subject: [PATCH] feat: a bind address you can set, because a container cannot use loopback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `docker-compose.yml` shipped a container nothing could reach. It published `127.0.0.1:8787:8787`, but paddock bound `127.0.0.1` INSIDE the container, and a published port is delivered to the container's own interface — so docker-proxy dialled an address the listener refused. Measured both ways on a spare port: loopback bind answers HTTP 000 through the publish, `0.0.0.0` answers 200 with herdr connected. So `PADDOCK_HOST`, defaulting to `127.0.0.1`. Empty or whitespace resolves to loopback rather than a wildcard: `PADDOCK_HOST:` left half-written in a compose file must not silently publish an unauthenticated dashboard. A non-loopback bind warns and proceeds. It cannot be an error — a container requires one — and it must not be silent, because decision 3 gives this listener no authentication at all, so reachability IS authority: anything that can open the port can read every agent's screen and type into it. The warning names the address and says that plainly. `isLoopbackBind` is deliberately NOT `origin.ts`'s `isLoopbackHost`. That one classifies `Host` headers, which name one machine and are never wildcards; `0.0.0.0` and `::` exist only as bind addresses. Sharing the predicate would put a seam in the thing that file exists to keep seamless. The banner no longer prints `http://0.0.0.0:8787`. It reads as a link and opens nowhere; a wildcard bind gets a loopback URL plus the fact of the wildcard. Decision 3 asserted paddock "binds loopback only", which is now false, so it gets a scope note rather than a silent contradiction — including that this adds no authentication and removes none, and that what protects a non-loopback bind is whatever sits in front of the port. Test data uses RFC 5737 documentation addresses: `check-clean` refuses private address literals in a public repo, and the predicate cannot tell them apart. Co-Authored-By: Claude Opus 5 --- .env.example | 14 ++++++ docker-compose.yml | 10 +++++ docs/decisions.md | 16 +++++++ src/server/index.ts | 22 ++++++++- src/server/startup-errors.ts | 87 ++++++++++++++++++++++++++++++++++++ tests/startup-errors.test.ts | 77 +++++++++++++++++++++++++++++++ 6 files changed, 224 insertions(+), 2 deletions(-) diff --git a/.env.example b/.env.example index 61db537..35bc5c9 100644 --- a/.env.example +++ b/.env.example @@ -5,6 +5,20 @@ PADDOCK_HOST_ID=local # PADDOCK_HERDR_SOCKET=/herdr.sock PADDOCK_PORT=8787 +# The address paddock BINDS. Loopback by default, and that default is load +# bearing: this listener has no authentication of its own (docs/decisions.md +# decision 3), so anything that can reach the port can read every agent's +# screen and type into it. +# +# Override it only where the port is already protected. The case it exists for +# is a container: a published port is delivered to the container's own +# interface, so a loopback bind refuses it and `make up` produces a container +# nothing can reach. docker-compose.yml sets this to 0.0.0.0 and pins the +# published port to 127.0.0.1, which is what keeps the dashboard off the LAN. +# +# A non-loopback bind prints a warning at startup naming the address. +# PADDOCK_HOST=127.0.0.1 + # Seeds ~/.config/paddock/settings.json on FIRST RUN ONLY. Once that file # exists the dashboard owns these values and this is ignored. # PADDOCK_TELEGRAM_TOKEN= diff --git a/docker-compose.yml b/docker-compose.yml index 4842132..2187b3f 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -6,6 +6,11 @@ services: user: "${UID}:${GID}" # 127.0.0.1 prefix is REQUIRED. The short form "8787:8787" publishes on every # interface and exposes the dashboard to the local network. + # + # This prefix and PADDOCK_HOST below are NOT the same control, and both are + # needed. PADDOCK_HOST is what the listener binds INSIDE the container; + # this prefix is which host interface the port is published ON. The prefix + # is the one keeping the dashboard off your network. ports: - "127.0.0.1:8787:8787" volumes: @@ -13,5 +18,10 @@ services: environment: PADDOCK_HERDR_SOCKET: /herdr.sock PADDOCK_PORT: "8787" + # REQUIRED in a container, and only in a container. A published port is + # delivered to the container's own interface, so paddock's default + # loopback bind refuses it and the published port reaches nothing. + # Safe here ONLY because the publish above is pinned to 127.0.0.1. + PADDOCK_HOST: "0.0.0.0" PADDOCK_HOST_ID: ${PADDOCK_HOST_ID:-local} restart: unless-stopped diff --git a/docs/decisions.md b/docs/decisions.md index 2ad424f..61de5c4 100644 --- a/docs/decisions.md +++ b/docs/decisions.md @@ -29,6 +29,22 @@ session does not silently re-litigate them. Access in front of it at all — see decision 13 for why that is not the mechanism this decision rules out. + Bind address, added later: the default listener binds loopback *by default*, + not unconditionally — `PADDOCK_HOST` can move it. That is not a softening of + this decision, because it adds no authentication and removes none: there was + never any. It exists because a container cannot use loopback at all. A + published port is delivered to the container's own interface, so a + loopback-bound listener refuses it, and `docker-compose.yml` shipped a + container nothing could reach for exactly that reason. + + What protects a non-loopback bind is therefore NOT paddock. It is whatever + sits in front of the port — the `127.0.0.1:` prefix on a compose publish, a + container network, a firewall. paddock's only contribution is to say so: a + non-loopback bind prints a warning naming the address and the fact that the + port has no authentication behind it. Do not read this entry as permission + to bind `0.0.0.0` on a workstation; on a shared network that hands every + device full control of the operator's agents. + 4. **`agent.list`, not `pane.list`.** Only `agent.list` returns the operator-assigned `name` field; `PaneInfo` has no `name` (it has `label`). Using `pane.list` is the difference between a useful dashboard and one diff --git a/src/server/index.ts b/src/server/index.ts index cc8e982..8ea30c3 100644 --- a/src/server/index.ts +++ b/src/server/index.ts @@ -41,13 +41,20 @@ import { herdrUnreachableMessage, inspectSocketPath, isDiagnosableHerdrFailure, + listeningLine, + nonLoopbackBindWarning, portInUseMessage, + resolveHost, } from "@server/startup-errors"; const { command, flags, values, verb } = parseArgs(Bun.argv.slice(2)); const DEMO = flags.has("--demo"); const PORT = Number(process.env.PADDOCK_PORT ?? 8787); -const HOSTNAME = "127.0.0.1"; // loopback only; exposure is the tunnel's job +// Loopback by default; exposure is normally the tunnel's job. `PADDOCK_HOST` +// exists for the one case that cannot use loopback: in a container, a published +// port arrives on the container's own interface, so a loopback listener refuses +// it. A non-loopback bind warns at startup — see `nonLoopbackBindWarning`. +const HOSTNAME = resolveHost(process.env); // Reserved, and routed through the parser like every other verb. This used to // scan raw `Bun.argv` instead — two argv mechanisms in one function, which is @@ -620,7 +627,18 @@ if (bootSummary !== null) console.info(bootSummary); bootLog.end(); say(""); -say(` paddock \`http://${HOSTNAME}:${PORT}\``); +say(listeningLine(HOSTNAME, PORT)); + +// After the URL, so it is the line the eye lands on last, and before the tunnel +// hint, which would otherwise separate the warning from what it is about. +const bindWarning = nonLoopbackBindWarning(HOSTNAME, PORT); +if (bindWarning !== null) { + say(""); + warn(bindWarning); + // The tunnel hint below is indented like the warning's own lines and reads as + // part of it otherwise. + say(""); +} // Nothing to nudge an operator who is already running `paddock tunnel` toward. if (command !== "tunnel") { diff --git a/src/server/startup-errors.ts b/src/server/startup-errors.ts index aea40c7..4b8932b 100644 --- a/src/server/startup-errors.ts +++ b/src/server/startup-errors.ts @@ -111,3 +111,90 @@ export function herdrUnreachableMessage( " no herdr at all: `paddock --demo` runs with synthetic agents", ].join("\n"); } + +/** + * Where the listener BINDS, and what the operator is told when that is not + * loopback. + * + * This is a different question from the one `origin.ts` answers, and the two + * must not share a predicate. `origin.ts` classifies `Host` HEADERS, which name + * one machine and are never wildcards. A bind address can be `0.0.0.0` or `::`, + * which name every interface at once — the case that matters here and the case + * a `Host` header cannot express. + * + * WHY THE DEFAULT MUST STAY LOOPBACK. `docs/decisions.md` decision 3 gives this + * listener no authentication of its own, deliberately, and the same-origin gate + * in `origin.ts` says so in as many words: it is a CSRF control, not an + * authenticator. So reachability IS authority here — anything that can open the + * port can read every agent's screen and type into it. Loopback is what makes + * that acceptable, which is why the override exists but announces itself. + */ + +/** Bind addresses that name only this machine, other than the `127.0.0.0/8` block. */ +const LOOPBACK_BINDS: readonly string[] = ["localhost", "::1", "[::1]"]; + +/** Bind addresses that mean "every interface". */ +const WILDCARD_BINDS: readonly string[] = ["0.0.0.0", "::", "[::]"]; + +/** + * The address to bind, from the environment. + * + * An empty or whitespace-only value is treated as UNSET rather than as a + * wildcard. `PADDOCK_HOST:` with nothing after it in a compose file, and + * `PADDOCK_HOST=` in an `.env`, both arrive here as `""` — and resolving that + * to `0.0.0.0` would silently publish an unauthenticated dashboard because + * somebody left a line half-written. + */ +export function resolveHost(env: Record): string { + const value = env.PADDOCK_HOST?.trim(); + return value === undefined || value === "" ? "127.0.0.1" : value; +} + +/** Whether a bind address reaches only this machine. */ +export function isLoopbackBind(host: string): boolean { + const name = host.trim().toLowerCase(); + if (LOOPBACK_BINDS.includes(name)) return true; + // The whole 127.0.0.0/8 block, not just 127.0.0.1 — a resolver stub on + // 127.0.0.53 is loopback too, and warning about it would be noise. + return /^127\.\d{1,3}\.\d{1,3}\.\d{1,3}$/.test(name); +} + +/** Whether a bind address names every interface. */ +function isWildcardBind(host: string): boolean { + return WILDCARD_BINDS.includes(host.trim().toLowerCase()); +} + +/** + * What the operator is told when the bind is not loopback, or `null` when it is. + * + * Not an error: a container REQUIRES a non-loopback bind, because published + * ports are delivered to the container's own interface and a loopback listener + * refuses them. So this warns and proceeds. It names the address and port + * literally, because the failure it is trying to prevent is someone believing + * this is still a private dashboard. + */ +export function nonLoopbackBindWarning(host: string, port: number): string | null { + if (isLoopbackBind(host)) return null; + return [ + `paddock: bound to ${host}:${port} — not loopback`, + " every host that can reach this port has full control of your agents:", + " it can read their screens and type into them. paddock has", + " no authentication of its own (docs/decisions.md decision 3).", + " In a container this is expected — keep the published port on 127.0.0.1.", + " On a desk it means your network can drive your agents.", + ].join("\n"); +} + +/** + * The banner line naming where the dashboard is. + * + * A wildcard bind gets a loopback URL plus the fact of the wildcard, never + * `http://0.0.0.0:8787` — that string looks like a link, and it is not one any + * browser can open. The banner's only job is to be clickable. + */ +export function listeningLine(host: string, port: number): string { + if (isWildcardBind(host)) { + return ` paddock \`http://127.0.0.1:${port}\` (all interfaces, port ${port})`; + } + return ` paddock \`http://${host}:${port}\``; +} diff --git a/tests/startup-errors.test.ts b/tests/startup-errors.test.ts index 05ff542..9569cf2 100644 --- a/tests/startup-errors.test.ts +++ b/tests/startup-errors.test.ts @@ -6,7 +6,11 @@ import { herdrUnreachableMessage, inspectSocketPath, isDiagnosableHerdrFailure, + isLoopbackBind, + listeningLine, + nonLoopbackBindWarning, portInUseMessage, + resolveHost, type SocketPathKind, } from "@server/startup-errors"; @@ -189,3 +193,76 @@ test("a failure paddock cannot diagnose keeps its own message, not a herdr one", true, ); }); + +// --------------------------------------------------------------------------- +// Where paddock BINDS, which is a different question from which `Host` headers +// it answers (`origin.ts`). A bind address can be a wildcard; a `Host` header +// never is, so the two predicates must not be shared. +// +// The default has to stay loopback. `docs/decisions.md` decision 3 gives this +// listener no authentication at all, so a wildcard bind reached from anywhere +// is full control of the operator's agents — which is why the override warns. + +test("resolveHost defaults to loopback when PADDOCK_HOST is unset", () => { + expect(resolveHost({})).toBe("127.0.0.1"); +}); + +test("resolveHost honours an explicit PADDOCK_HOST", () => { + expect(resolveHost({ PADDOCK_HOST: "0.0.0.0" })).toBe("0.0.0.0"); +}); + +// A compose file with `PADDOCK_HOST:` and no value, or `PADDOCK_HOST=` in an +// .env, must not bind a wildcard by accident — it means "unset", not "any". +test("resolveHost treats an empty or whitespace PADDOCK_HOST as unset", () => { + expect(resolveHost({ PADDOCK_HOST: "" })).toBe("127.0.0.1"); + expect(resolveHost({ PADDOCK_HOST: " " })).toBe("127.0.0.1"); +}); + +test("resolveHost trims surrounding whitespace", () => { + expect(resolveHost({ PADDOCK_HOST: " 0.0.0.0 " })).toBe("0.0.0.0"); +}); + +test("isLoopbackBind accepts every spelling of loopback", () => { + for (const host of ["127.0.0.1", "127.0.0.53", "localhost", "::1", "[::1]"]) { + expect(isLoopbackBind(host)).toBe(true); + } +}); + +// The wildcards are the whole point: `0.0.0.0` is what a bridge-network +// container needs, and it is also what exposes a desk to its LAN. +// +// The routable examples are RFC 5737 documentation addresses, standing in for +// the RFC1918 ranges a container bridge and a home network actually use. +// `make check-clean` refuses private address literals in a public repo, and it +// is right to: the predicate under test cannot tell the two apart, so the test +// loses nothing by not naming a real network. +test("isLoopbackBind refuses wildcards and routable addresses", () => { + for (const host of ["0.0.0.0", "::", "[::]", "203.0.113.5", "198.51.100.7"]) { + expect(isLoopbackBind(host)).toBe(false); + } +}); + +test("nonLoopbackBindWarning stays silent for a loopback bind", () => { + expect(nonLoopbackBindWarning("127.0.0.1", 8787)).toBeNull(); +}); + +test("nonLoopbackBindWarning names the address, the port and the risk", () => { + const warning = nonLoopbackBindWarning("0.0.0.0", 8787); + expect(warning).not.toBeNull(); + expect(warning).toContain("0.0.0.0:8787"); + // The operator has to be told the thing that is actually true: there is no + // authentication behind this port. + expect(warning).toContain("no authentication"); +}); + +// `http://0.0.0.0:8787` is not a URL anyone can open, so a wildcard bind must +// not print one — it was the banner's only job to be clickable. +test("listeningLine prints a clickable URL for a specific host", () => { + expect(listeningLine("127.0.0.1", 8787)).toContain("http://127.0.0.1:8787"); +}); + +test("listeningLine does not offer a wildcard as a URL", () => { + const line = listeningLine("0.0.0.0", 8787); + expect(line).not.toContain("http://0.0.0.0"); + expect(line).toContain("all interfaces"); +});