Skip to content

Commit 2be9310

Browse files
aaronjmarsclaude
andauthored
fix(security): validate Host header on /api/* to block DNS rebinding (#61)
* fix(security): validate Host header on /api/* to block DNS rebinding `next dev` and `next start` bind to 0.0.0.0 by default. A malicious page can DNS-rebind an attacker-controlled name (`attacker.example` → `127.0.0.1`) and POST to `/api/convert`, `/api/deploy`, etc. through the user's browser — `/api/convert` spawns the local agent CLI with maximally permissive flags, so a successful forged-Host POST is unauthenticated RCE via the agent. Add a Next middleware that gates every `/api/*` request on a Host-header allowlist. Defaults to loopback only (`127.0.0.1`, `localhost`, `::1`, any port). Two operator knobs: - `HTML_ANYTHING_ALLOWED_HOSTS=host1,host2,…` — extend the allowlist for LAN / mDNS / `.local` setups. - `HTML_ANYTHING_ALLOW_ANY_HOST=1` — bypass entirely, for when a trusted reverse proxy is terminating Host upstream. Loudly insecure by design; not the default. Restructured for the workspace layout per maintainer guidance on PR #61: - `next/src/middleware.ts` — the Next middleware (runs on `/api/:path*`) - `next/src/lib/security/host-validation.ts` — pure validator + env wrapper - `next/src/lib/security/host-validation.test.ts` — vitest, runs under `pnpm -F @html-anything/next test` (`src/**/*.test.ts` glob) - `e2e/ui/host-validation.spec.ts` — Playwright, runs under `pnpm -F @html-anything/e2e test`. Covers the accept-loopback path so the default `next start -p 3317` UX still works, plus the reject path for attacker.example / subdomain tricks / forged POSTs against /api/convert + /api/deploy/config. - README.md — new `## Security` section documenting when to use the defaults vs `ALLOWED_HOSTS` vs `ALLOW_ANY_HOST=1` (operator story). Root `package.json` left untouched (zero scripts, workspace metadata only) per the workspace rule in `AGENTS.md`. No source under root `src/` / `app/`, no Playwright outside `e2e/`. * fix(security): tighten loopback allowlist + pin middleware to Node runtime Addresses the three findings from @PerishCode's review on #61: 1. **Drop `0.0.0.0` from the loopback allowlist** — on macOS/Linux it routes to the local machine, and pre-fix Chrome (< 128) lets a public page fetch `http://0.0.0.0:<port>` directly, bypassing DNS rebinding entirely. `LOOPBACK_HOSTS.has("0.0.0.0")` would have returned true and reached `/api/convert` (the agent-spawn RCE path). The justifying comment ("some test runners send 0.0.0.0 as Host") doesn't hold for this repo — `e2e/playwright.config.ts` dials `127.0.0.1:3317`, no test sent 0.0.0.0. 2. **Drop bare `::1` from the loopback allowlist** — `stripPort("::1")` produces `":"` (the last-colon-trailing-digit branch), so bare `::1` could never match anyway. Only the bracketed `[::1]` form is reachable, and that's what browsers and HTTP/2 `:authority` actually send. Added a `stripPort("::1") === ":"` assertion to document the behavior. 3. **Pin middleware to Node runtime** — `export const runtime = "nodejs"` in `middleware.ts`. The `HTML_ANYTHING_ALLOWED_HOSTS` / `HTML_ANYTHING_ALLOW_ANY_HOST` env knobs are read by `isRequestHostAllowed` inside the middleware; on Edge runtime Next can inline `process.env.*` references at build time, which would silently fail to extend the allowlist (lock-out) or fail to disable the gate (false reassurance). Node runtime middleware (Next 15.2+; this repo is on 16.2.6) reads env per-request. README `## Security` updated to call out the choice. Unit tests extended: - `stripPort("::1") === ":"` — documents the IPv6-bare mangle. - `isAllowedHost("0.0.0.0") === false` and `isAllowedHost("0.0.0.0:3317") === false`. - `isAllowedHost("::1") === false` — bare unbracketed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(e2e): provide baseURL fallback under exactOptionalPropertyTypes @PerishCode flagged that `e2e/ui/host-validation.spec.ts` fails the "Typecheck e2e" CI step. The `baseURL` fixture is `string | undefined`, e2e/tsconfig.json sets `exactOptionalPropertyTypes: true`, and `newContext`'s `baseURL?: string` rejects an explicit undefined under that flag — tsc errored at all seven call sites. Hoists a `DEFAULT_BASE_URL` matching `webServer.url` in e2e/playwright.config.ts (`http://127.0.0.1:3317`) and uses `baseURL ?? DEFAULT_BASE_URL` at every newContext site. Runtime behavior is unchanged when baseURL is set (which it always is when Playwright runs the suite); the fallback only satisfies the type checker for the undefined branch. Verified locally: pnpm -F @html-anything/e2e typecheck # clean Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(e2e): send single-space Host to deterministically test empty-Host branch @PerishCode flagged that `Host: ""` is at the mercy of Playwright's header serialization — if the empty value is dropped, the request goes out with the default loopback Host and the test passes for the wrong reason; if it's transmitted, the validator deterministically returns 403. Outcome depended on undefined HTTP-stack behavior. Switched to `Host: " "`. Per RFC 7230 a single-space header value is transmitted, the receiving parser strips the surrounding OWS, and the server sees `host: ""` deterministically. Either way the validator's `stripPort.trim()` reduces it to "" and `isAllowedHost("")` returns false → 403 — the test now exercises the empty-Host branch it claims to, with no flake surface. Verified locally: `pnpm -F @html-anything/e2e typecheck` clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 145a40e commit 2be9310

5 files changed

Lines changed: 459 additions & 0 deletions

File tree

README.md

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -440,6 +440,18 @@ Early but real. The closed loop — **detect agent → pick skill → SSE stream
440440
| History / version diff / IndexedDB archive | ⏳ planned |
441441
| Skill marketplace (`install <github-repo>`) | ⏳ planned |
442442

443+
## Security
444+
445+
The Next API surface is the local-only side of the app — `/api/convert` spawns the user's coding-agent CLI with maximally permissive flags, `/api/deploy` writes credentialed config to disk. Both are intended for a single operator on a single machine. To prevent a malicious page from DNS-rebinding `attacker.example` to `127.0.0.1` and POSTing into those routes through the user's browser, every `/api/*` request is gated on a Host-header allowlist in [`next/src/middleware.ts`](next/src/middleware.ts).
446+
447+
| Setting | When to use |
448+
|---|---|
449+
| **Default (no env vars set)** | The common case — `next dev` / `next start` on your own machine. `127.0.0.1`, `localhost`, and `::1` Host headers (any port) are accepted. Everything else gets a 403 `{ "error": "Host not allowed" }`. |
450+
| **`HTML_ANYTHING_ALLOWED_HOSTS=daemon.mirage.local,html-anything.lan`** | LAN / mDNS setup — you're reaching the app from another device on the same network and the browser dials a non-loopback hostname. Comma-separated; port-insensitive; case-insensitive. The default loopback set is still accepted on top of this. |
451+
| **`HTML_ANYTHING_ALLOW_ANY_HOST=1`** | Reverse-proxy mode — Caddy / nginx / Cloudflare Tunnel is terminating the public hostname and forwarding to the app. The proxy is now responsible for Host policy. Loudly insecure if you set this without a trusted proxy in front, so it is not the default. |
452+
453+
Set the env var in whatever environment file your launcher reads (e.g. `next/.env.local`). The middleware is pinned to the Node runtime (`export const runtime = "nodejs"` in [`next/src/middleware.ts`](next/src/middleware.ts)) so `process.env` is read per-request — Edge middleware can inline `process.env.*` at build time, which would silently break operator overrides set after `next build`. The validation is unit-tested in [`next/src/lib/security/host-validation.test.ts`](next/src/lib/security/host-validation.test.ts) and the loopback-still-works dev path is exercised in the Playwright spec at [`e2e/ui/host-validation.spec.ts`](e2e/ui/host-validation.spec.ts).
454+
443455
## Contributing
444456

445457
Issues, PRs, new skills, new agent adapters, new export targets, and translations are all welcome. The highest-leverage contributions are usually **one folder, one Markdown file, or one PR-sized adapter** — small surface area, big leverage. Pick the slot that matches what you want to add:

e2e/ui/host-validation.spec.ts

Lines changed: 131 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,131 @@
1+
/**
2+
* End-to-end verification that the API surface refuses requests whose `Host`
3+
* header isn't on the loopback allowlist. The exact rebinding attack we are
4+
* defending against would deliver these requests from a real browser tab
5+
* after a DNS flip; here we use a raw `request.fetch` so we can forge the
6+
* header directly.
7+
*
8+
* The Playwright webServer in `e2e/playwright.config.ts` runs
9+
* `next start -p 3317`, which binds to every interface by default. The
10+
* default `baseURL` resolves to `127.0.0.1:3317` — the loopback path. To
11+
* exercise the rejection path we override the `Host` header to attacker-
12+
* controlled values while still dialing the real loopback IP.
13+
*
14+
* The accept-loopback tests prove the default dev path keeps working —
15+
* gating on Host must not break the common case where the user opens
16+
* `http://localhost:3317/` in a browser on their own machine.
17+
*/
18+
import { test, expect, request as playwrightRequest } from "@playwright/test";
19+
20+
const API_PATHS = [
21+
"/api/agents",
22+
"/api/templates",
23+
"/api/deploy/config?provider=vercel",
24+
] as const;
25+
26+
// Matches the `webServer.url` in e2e/playwright.config.ts (port 3317). Used as
27+
// a fallback when the `baseURL` fixture is undefined — exactOptionalPropertyTypes
28+
// rejects passing `string | undefined` to newContext's `baseURL?: string`.
29+
const DEFAULT_BASE_URL = "http://127.0.0.1:3317";
30+
31+
test.describe("API host-header validation", () => {
32+
test("accepts loopback Host (127.0.0.1) — default dev path", async ({ baseURL }) => {
33+
const ctx = await playwrightRequest.newContext({
34+
baseURL: baseURL ?? DEFAULT_BASE_URL,
35+
extraHTTPHeaders: { Host: "127.0.0.1:3317" },
36+
});
37+
for (const p of API_PATHS) {
38+
const r = await ctx.get(p);
39+
// Any 2xx / 4xx that ISN'T 403 from this middleware passes — the route
40+
// may legitimately 400 (missing query / not configured) but it should
41+
// not be the host-rejection 403.
42+
if (r.status() === 403) {
43+
const body = await r.json().catch(() => ({}));
44+
expect(body.error, `loopback host rejected on ${p}: ${JSON.stringify(body)}`).not.toBe(
45+
"Host not allowed",
46+
);
47+
}
48+
}
49+
await ctx.dispose();
50+
});
51+
52+
test("accepts localhost Host — default dev path", async ({ baseURL }) => {
53+
const ctx = await playwrightRequest.newContext({
54+
baseURL: baseURL ?? DEFAULT_BASE_URL,
55+
extraHTTPHeaders: { Host: "localhost:3317" },
56+
});
57+
const r = await ctx.get("/api/agents");
58+
expect(r.status()).not.toBe(403);
59+
await ctx.dispose();
60+
});
61+
62+
test("rejects attacker.example Host on every API path", async ({ baseURL }) => {
63+
const ctx = await playwrightRequest.newContext({
64+
baseURL: baseURL ?? DEFAULT_BASE_URL,
65+
extraHTTPHeaders: { Host: "attacker.example" },
66+
});
67+
for (const p of API_PATHS) {
68+
const r = await ctx.get(p);
69+
expect(r.status(), `${p} should reject attacker.example`).toBe(403);
70+
const body = await r.json();
71+
expect(body.error).toBe("Host not allowed");
72+
}
73+
await ctx.dispose();
74+
});
75+
76+
test("rejects POST /api/convert with attacker Host (RCE vector)", async ({ baseURL }) => {
77+
const ctx = await playwrightRequest.newContext({
78+
baseURL: baseURL ?? DEFAULT_BASE_URL,
79+
extraHTTPHeaders: { Host: "attacker.example" },
80+
});
81+
const r = await ctx.post("/api/convert", {
82+
data: { agent: "claude", templateId: "deck-swiss-international", content: "ignore" },
83+
});
84+
expect(r.status(), "POST /api/convert must reject forged Host (RCE vector)").toBe(403);
85+
await ctx.dispose();
86+
});
87+
88+
test("rejects PUT /api/deploy/config with attacker Host (token-write vector)", async ({
89+
baseURL,
90+
}) => {
91+
const ctx = await playwrightRequest.newContext({
92+
baseURL: baseURL ?? DEFAULT_BASE_URL,
93+
extraHTTPHeaders: { Host: "attacker.example" },
94+
});
95+
const r = await ctx.put("/api/deploy/config?provider=vercel", {
96+
data: { token: "attacker-token" },
97+
});
98+
expect(r.status()).toBe(403);
99+
await ctx.dispose();
100+
});
101+
102+
test("rejects subdomain-tricks (localhost.attacker.example)", async ({ baseURL }) => {
103+
const ctx = await playwrightRequest.newContext({
104+
baseURL: baseURL ?? DEFAULT_BASE_URL,
105+
extraHTTPHeaders: { Host: "localhost.attacker.example" },
106+
});
107+
const r = await ctx.get("/api/agents");
108+
expect(r.status()).toBe(403);
109+
await ctx.dispose();
110+
});
111+
112+
test("rejects empty Host", async ({ baseURL }) => {
113+
// A single space is deterministically transmitted; per RFC 7230 the
114+
// receiving parser strips the surrounding OWS and the server sees an
115+
// empty `host`. The validator's `stripPort.trim()` reduces it to "" and
116+
// `isAllowedHost("")` returns false → 403. An empty-string value would
117+
// be at the mercy of Playwright's header-serialization behavior (it may
118+
// drop the entry, in which case the request goes out with the default
119+
// loopback Host and the test passes for the wrong reason).
120+
const ctx = await playwrightRequest.newContext({
121+
baseURL: baseURL ?? DEFAULT_BASE_URL,
122+
extraHTTPHeaders: { Host: " " },
123+
});
124+
const r = await ctx.get("/api/agents");
125+
// Some HTTP stacks reject empty Host headers themselves with 400 before
126+
// it reaches middleware. Either 400 or 403 is acceptable; the key
127+
// invariant is "no 200".
128+
expect([400, 403]).toContain(r.status());
129+
await ctx.dispose();
130+
});
131+
});
Lines changed: 146 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,146 @@
1+
import { afterEach, describe, expect, it } from "vitest";
2+
import {
3+
isAllowedHost,
4+
isRequestHostAllowed,
5+
parseAllowedHosts,
6+
stripPort,
7+
} from "./host-validation";
8+
9+
describe("stripPort", () => {
10+
it("preserves bare hostnames", () => {
11+
expect(stripPort("localhost")).toBe("localhost");
12+
expect(stripPort("127.0.0.1")).toBe("127.0.0.1");
13+
expect(stripPort("daemon.mirage.local")).toBe("daemon.mirage.local");
14+
});
15+
it("strips ipv4 + dns ports", () => {
16+
expect(stripPort("localhost:3000")).toBe("localhost");
17+
expect(stripPort("127.0.0.1:3317")).toBe("127.0.0.1");
18+
expect(stripPort("example.com:443")).toBe("example.com");
19+
});
20+
it("strips ipv6 ports while keeping brackets", () => {
21+
expect(stripPort("[::1]:3000")).toBe("[::1]");
22+
expect(stripPort("[::1]")).toBe("[::1]");
23+
});
24+
it("lower-cases", () => {
25+
expect(stripPort("LOCALHOST:3000")).toBe("localhost");
26+
});
27+
it("does not strip when the trailing chunk is not a port", () => {
28+
expect(stripPort("not-a-port:abc")).toBe("not-a-port:abc");
29+
});
30+
// Documents the fact that bare unbracketed `::1` is mangled by the
31+
// `last-colon-trailing-digit` branch — the last colon is index 1 and the
32+
// trailing "1" is all digits, so the slice returns ":". A bare `::1` can
33+
// therefore never match anything in `LOOPBACK_HOSTS`, which is why only
34+
// the bracketed `[::1]` form is on the allowlist.
35+
it("mangles bare ::1 to ':' (only [::1] is a real Host header anyway)", () => {
36+
expect(stripPort("::1")).toBe(":");
37+
});
38+
});
39+
40+
describe("parseAllowedHosts", () => {
41+
it("returns an empty set for undefined / empty input", () => {
42+
expect(parseAllowedHosts(undefined).size).toBe(0);
43+
expect(parseAllowedHosts("").size).toBe(0);
44+
expect(parseAllowedHosts(" , ,").size).toBe(0);
45+
});
46+
it("splits + trims + lowercases + strips ports", () => {
47+
const set = parseAllowedHosts("Daemon.mirage.local, HOST-A:8080 , host-b");
48+
expect([...set].sort()).toEqual(["daemon.mirage.local", "host-a", "host-b"]);
49+
});
50+
});
51+
52+
describe("isAllowedHost (defaults — loopback only)", () => {
53+
it("accepts loopback variants on any port", () => {
54+
expect(isAllowedHost("127.0.0.1")).toBe(true);
55+
expect(isAllowedHost("127.0.0.1:3000")).toBe(true);
56+
expect(isAllowedHost("localhost")).toBe(true);
57+
expect(isAllowedHost("LOCALHOST:3317")).toBe(true);
58+
expect(isAllowedHost("[::1]:3000")).toBe(true);
59+
expect(isAllowedHost("[::1]")).toBe(true);
60+
});
61+
it("rejects attacker hosts", () => {
62+
expect(isAllowedHost("attacker.example")).toBe(false);
63+
expect(isAllowedHost("attacker.example:80")).toBe(false);
64+
expect(isAllowedHost("evil.local")).toBe(false);
65+
// Adjacent loopback aliases that aren't on the allowlist — keep strict
66+
expect(isAllowedHost("127.0.0.2")).toBe(false);
67+
expect(isAllowedHost("localhost.attacker.example")).toBe(false);
68+
});
69+
// `0.0.0.0` is reachable from a public page on pre-fix Chrome (< 128) — it
70+
// routes to the local machine on macOS/Linux without needing DNS rebinding.
71+
// Must be rejected so the gate covers that sibling vector.
72+
it("rejects 0.0.0.0 (sidesteps DNS rebinding via 0.0.0.0-day vector)", () => {
73+
expect(isAllowedHost("0.0.0.0")).toBe(false);
74+
expect(isAllowedHost("0.0.0.0:3317")).toBe(false);
75+
});
76+
// Bare unbracketed `::1` is mangled by stripPort (see stripPort tests) and
77+
// browsers / HTTP/2 always bracket IPv6 in the Host / :authority field.
78+
it("rejects bare unbracketed ::1 (only [::1] is a real Host header)", () => {
79+
expect(isAllowedHost("::1")).toBe(false);
80+
});
81+
it("rejects empty / missing host", () => {
82+
expect(isAllowedHost(null)).toBe(false);
83+
expect(isAllowedHost(undefined)).toBe(false);
84+
expect(isAllowedHost("")).toBe(false);
85+
expect(isAllowedHost(" ")).toBe(false);
86+
});
87+
});
88+
89+
describe("isAllowedHost — operator-extended allowlist", () => {
90+
const extras = parseAllowedHosts("daemon.mirage.local,html.anything.lan");
91+
it("accepts entries from extraAllowed (case + port insensitive)", () => {
92+
expect(isAllowedHost("daemon.mirage.local", { extraAllowed: extras })).toBe(true);
93+
expect(isAllowedHost("DAEMON.MIRAGE.LOCAL:3000", { extraAllowed: extras })).toBe(true);
94+
expect(isAllowedHost("html.anything.lan:8080", { extraAllowed: extras })).toBe(true);
95+
});
96+
it("still rejects non-listed hosts even when extras are configured", () => {
97+
expect(isAllowedHost("attacker.example", { extraAllowed: extras })).toBe(false);
98+
});
99+
it("accepts a string[] form for extraAllowed (not just Set)", () => {
100+
expect(
101+
isAllowedHost("daemon.mirage.local", { extraAllowed: ["daemon.mirage.local"] }),
102+
).toBe(true);
103+
});
104+
});
105+
106+
describe("isAllowedHost — wildcard opt-out", () => {
107+
it("allowAny=true accepts any host (reverse-proxy mode)", () => {
108+
expect(isAllowedHost("attacker.example", { allowAny: true })).toBe(true);
109+
expect(isAllowedHost(null, { allowAny: true })).toBe(true);
110+
expect(isAllowedHost("", { allowAny: true })).toBe(true);
111+
});
112+
});
113+
114+
describe("isRequestHostAllowed (env-driven wrapper)", () => {
115+
const make = (host: string | null) => ({
116+
headers: {
117+
get(name: string) {
118+
return name.toLowerCase() === "host" ? host : null;
119+
},
120+
},
121+
});
122+
123+
afterEach(() => {
124+
delete process.env.HTML_ANYTHING_ALLOWED_HOSTS;
125+
delete process.env.HTML_ANYTHING_ALLOW_ANY_HOST;
126+
});
127+
128+
it("respects defaults when no env is set", () => {
129+
expect(isRequestHostAllowed(make("127.0.0.1:3317"))).toBe(true);
130+
expect(isRequestHostAllowed(make("attacker.example"))).toBe(false);
131+
expect(isRequestHostAllowed(make(null))).toBe(false);
132+
});
133+
it("extends allowlist via HTML_ANYTHING_ALLOWED_HOSTS", () => {
134+
process.env.HTML_ANYTHING_ALLOWED_HOSTS = "html.anything.lan";
135+
expect(isRequestHostAllowed(make("html.anything.lan:3000"))).toBe(true);
136+
expect(isRequestHostAllowed(make("attacker.example"))).toBe(false);
137+
});
138+
it("opt-out wildcard via HTML_ANYTHING_ALLOW_ANY_HOST=1 accepts everything", () => {
139+
process.env.HTML_ANYTHING_ALLOW_ANY_HOST = "1";
140+
expect(isRequestHostAllowed(make("attacker.example"))).toBe(true);
141+
});
142+
it("envVar=0 stays strict (only '1' opts out)", () => {
143+
process.env.HTML_ANYTHING_ALLOW_ANY_HOST = "0";
144+
expect(isRequestHostAllowed(make("attacker.example"))).toBe(false);
145+
});
146+
});

0 commit comments

Comments
 (0)