Repository navigation
Validate the Host header on every request (defense in depth) - #1742
Merged
Merged
Conversation
Every plan, review, annotate, archive and goal-setup server, in both the Bun and Pi runtimes, now checks the request's Host header before any route (app HTML, API, WebSocket upgrades). Loopback names are always accepted; remote mode also accepts IP literals, the configured urlHost and the machine's hostname; a --tailscale session accepts its serve name. Other hostnames get a 403 naming PLANNOTATOR_ALLOWED_HOSTS, the new escape hatch for proxies and tunnels that forward their own hostname. The port is not compared, so forwarded ports keep working.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
…ntries
- Allow the per-port hostname of a code-server / Coder port proxy, read from
the `{{port}}` template in the VSCODE_PROXY_URI the IDE sets (any mode:
such a proxy reaches a loopback-bound server and forwards its own Host).
Only digits in the template's place match; a path-based template adds
nothing.
- Remote mode also answers `<first label>.local`, the mDNS name another LAN
device types (os.hostname() omits it on Linux and often on macOS).
- A `:port` on a PLANNOTATOR_ALLOWED_HOSTS entry is ignored instead of
silently voiding the entry.
- Docs: Codespaces and VS Code dev tunnels present `Host: localhost:<port>`,
so they need nothing; drop them as examples of forwarded hostnames and say
why no provider domain is built in. Note the agent-terminal sidecar.
| // A WebSocket upgrade never succeeds: a 403, or (node:http with no | ||
| // upgrade listener attached, i.e. no agent terminal) a closed | ||
| // socket. The guarded-listener case is request-host-guard.test.ts. | ||
| const upgrade = await rawRequest(`${loopbackBase(server)}/api/agent-terminal/pty/x`, { |
backnotprop
added a commit
that referenced
this pull request
Oct 7, 2026
…d (no UI yet) (#1745) * Plannotator Inbox, step 1: server, store, MCP and the inbox subcommand - `plannotator inbox` (person: start here and open the browser, or open the running one), `--background` (detached, prints the URL, never a tab), `--no-open`, and `plannotator inbox mcp` (stdio JSON-RPC proxy to /mcp that re-reads inbox/inbox.json per message and starts a stopped Inbox). - packages/server/inbox.ts: Bun-only, always 127.0.0.1, last port first then random, never 19432; health, list model, thread, picks, Send, resolve, SSE with a cursor, stateless /mcp, token-guarded stop route. - packages/shared/inbox/: append-only JSONL store (one seq across the Inbox, torn-last-line recovery, idempotent sends), registry (0600, token rotated per start, kept after exit). - packages/core/inbox-*.ts: wire shapes (Workspaces' Question + QuestionAnswer v1), question parsing with Stopped/Holds up, derived state, Send text. - MCP tools: send_message, read_thread, resolve_message, wait_for_reply (@modelcontextprotocol/server pinned 2.2.0). - Security: Host allowlist (local rule), no Origin on /mcp, same-origin + serverSession on writes, bearer token on connection routes, no CORS. - uninstall --purge stops a running Inbox first; inbox in the purge list. - DAST scans a seeded Inbox; the target also allows its own Host name, which the Host allowlist (#1742) otherwise refuses. - Skill reference and AGENTS.md (Server Runtimes exception, Inbox section, Inbox Server API table). - Proofs: live server tests, SDK client over HTTP and stdio, the subcommand as real processes, fast-check store properties. * inbox store: a failed Send never leaves answers sent without a reply; keys never collide - sendReply writes the questions' sent lines before the reply line. When the reply append fails, the questions are put back in memory; at load, a question naming a reply that does not exist reads as picked again. Before, the person was stuck on question_already_sent and the agent got nothing. - A failed append marks the file torn, so the next line starts fresh. - Two same-named roots whose 6-hex project keys collide get a longer hash instead of sharing one folder (the second project.json replaced the first). - The round-trip property now sends arbitrary text (newlines, quotes, NULs, U+2028, lone surrogates) and a crafted JSONL-injection body. * inbox: never start a second writer beside a busy Inbox; cheaper update probe; edge fixes - A live pid whose port takes the health request but does not answer (a paused or blocked Inbox) read as stopped, so the shim and --background started a second Inbox on the same store: proven live with SIGSTOP, both processes then wrote duplicate seqs. inboxStatus now reports it as busy; callers wait for it (20 s, then an error) and never replace it, and uninstall --purge treats it as still running. - Restart to update ran the compiled binary with --version every 60 s (~0.2 s CPU, ~440 MB peak RSS each time). It now stats the binary and runs it only when its identity changed. - The detached Inbox runs in inbox/ (not the starter's project folder, which Windows would refuse to delete) with windowsHide. - An HTTP/1.0 request with no Host (allowed by the Host guard) threw in new URL() and answered 500; it is read against this server now. - wait_for_reply on an already-resolved thread answers resolved at once instead of holding the call for 50 s. - DAST target: binaryPath null (no update probe of the bun binary).
1 task
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Every Plannotator server now checks the request's
Hostheader before routing anything: plan review (including archive mode), code review, annotate, goal setup, and the servers OpenCode embeds, in both the Bun runtime (packages/server) and the Pinode:httpmirror. The check covers the app HTML, every API endpoint, and WebSocket upgrades.Until now only the session-bridge and host-control endpoints checked
Host; the rest relied onisSameOriginOrNoOrigin, which comparesOriginwithHostand so cannot tell the session's own page from a page served under some other DNS name that resolves to this machine. This adds a server-wide hostname allowlist as an additional layer.isSameOriginOrNoOriginand the narrower bridge/host-control checks are unchanged.The rule
One pure predicate,
isAllowedRequestHostinpackages/shared/request-host.ts(vendored to Pi):localhost,*.localhost,127.0.0.0/8literals,[::1]PLANNOTATOR_REMOTE/ SSH, wide bind)urlHost(autoresolved through the existing cached lookup), and this machine'sos.hostname()(full name and first label)--tailscale(Bun CLI)tailscale serveMagicDNS name, registered once serve reports its URL.tailscale serveforwards the browser's ownHostto TCP backends, so the server sees the tailnet namePLANNOTATOR_ALLOWED_HOSTS(new).example.comalso matches subdomains;*turns the check offHostheader (an HTTP/1.0 non-browser client) is accepted. A malformedHostis refused.ssh -L, VS Code/devcontainer auto-forwarding to a loopback-bound server when the local port is taken, anddocker -p. A foreign hostname is refused based on the hostname alone.403with a shorttext/plainbody (X-Plannotator-Error: host_not_allowed). The body names the hostname andPLANNOTATOR_ALLOWED_HOSTS. It is never a 404, so it can't be confused with the app catch-all.Compatibility notes
Hostto the upstream'slocalhost:<port>, which is a loopback name.vscode-proxy-writes.test.tspasses in both runtimes.localhostor an IP address, which are accepted.PLANNOTATOR_ALLOWED_HOSTS. The 403 body says so.node:httphands an upgrade to everyupgradelistener, including the annotate agent terminal's. The Pi adapter therefore intercepts theupgradeevent itself, so a refused request never reaches a listener.Docs
CLAUDE.md: a new "Host allowlist" subsection under Server API, aPLANNOTATOR_ALLOWED_HOSTSenv row, and a note in the Tailnet sectionTests
packages/shared/request-host.test.tscovers the predicate:*.localhost, look-alike namesHost, the env list and*packages/server/request-host.scenarios.tsruns against both runtimes, throughpackages/server/request-host.test.tsandapps/pi-extension/server/request-host.test.ts, for the plan, review and annotate servers:localhost:<port>,127.0.0.1:<port>and[::1]:<port>are served./,/api/planor/api/diff,/api/doc,/api/external-annotations,/api/draft, the decision endpoint (/api/approveor/api/feedback),POST /api/config, and a WebSocket upgrade.PLANNOTATOR_ALLOWED_HOSTSworks, and*turns the check off.--tailscaleserve name.request-host-guard.test.tschecks that a refused upgrade never reaches anupgradelistener. I also checked this under real Node 24: the refused upgrade gets the 403 and the allowed upgrade completes.pi-session-bridge.test.tsis updated. A foreignHostis now refused by the server-wide check. The bridge's own loopback-and-port check is still exercised withlocalhost:<other port>.bun test apps/opencode-plugin apps/pi-extension apps/hook packages: 6266 pass, 0 fail.bun run typecheck: clean.apps/reviewandapps/hook, then ranbun apps/hook/server/index.ts annotate <tmpfile>with a temp data dir.Host: localhost:<port>and127.0.0.1:<port>got 200 on/and/api/plan;Host: evil.example:<port>got 403.