Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[wrangler] Backport recent inspector proxy server changes to V2 #4587

Merged
merged 2 commits into from
Dec 12, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/warm-dryers-double.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"wrangler": patch
---

Change dev registry and inspector server to listen on 127.0.0.1 instead of all interfaces
7 changes: 7 additions & 0 deletions .changeset/wise-seas-press.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"wrangler": patch
---

fix: validate `Host` and `Orgin` headers where appropriate

`Host` and `Origin` headers are now checked when connecting to the inspector proxy. If these don't match what's expected, the request will fail.
2 changes: 1 addition & 1 deletion packages/wrangler/src/__tests__/api-devregistry.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ describe("multi-worker testing", () => {
});

it("parentWorker and childWorker should be added devRegistry", async () => {
const resp = await fetch("http://localhost:6284/workers");
const resp = await fetch("http://127.0.0.1:6284/workers");
if (resp) {
const parsedResp = (await resp.json()) as {
parent: unknown;
Expand Down
14 changes: 7 additions & 7 deletions packages/wrangler/src/dev-registry.ts
Original file line number Diff line number Diff line change
@@ -1,17 +1,17 @@
import http from "http";
import net from "net";
import { createServer } from "node:http";
import bodyParser from "body-parser";
import express from "express";
import { createHttpTerminator } from "http-terminator";
import { fetch } from "undici";
import { logger } from "./logger";

import type { Config } from "./config";
import type { Server } from "http";
import type { HttpTerminator } from "http-terminator";
import type { Server } from "node:http";

const DEV_REGISTRY_PORT = "6284";
const DEV_REGISTRY_HOST = `http://localhost:${DEV_REGISTRY_PORT}`;
const DEV_REGISTRY_PORT = 6284;
const DEV_REGISTRY_HOST = `http://127.0.0.1:${DEV_REGISTRY_PORT}`;

let server: Server | null;
let terminator: HttpTerminator;
Expand Down Expand Up @@ -48,7 +48,7 @@ async function isPortAvailable() {
netServer.close();
resolve(true);
});
netServer.listen(DEV_REGISTRY_PORT);
netServer.listen(DEV_REGISTRY_PORT, "127.0.0.1");
});
}

Expand Down Expand Up @@ -80,9 +80,9 @@ export async function startWorkerRegistry() {
workers = {};
res.json(null);
});
server = http.createServer(app);
server = createServer(app);
terminator = createHttpTerminator({ server });
server.listen(DEV_REGISTRY_PORT);
server.listen(DEV_REGISTRY_PORT, "127.0.0.1");

/**
* The registry server may have already been started by another wrangler process.
Expand Down
53 changes: 49 additions & 4 deletions packages/wrangler/src/inspect.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,46 @@ interface InspectorProps {
name?: string;
}

const ALLOWED_HOST_HOSTNAMES = ["127.0.0.1", "[::1]", "localhost"];
const ALLOWED_ORIGIN_HOSTNAMES = [
"devtools.devprod.cloudflare.dev",
"cloudflare-devtools.pages.dev",
/^[a-z0-9]+\.cloudflare-devtools\.pages\.dev$/,
"127.0.0.1",
"[::1]",
"localhost",
];
function validateWebSocketHandshake(req: IncomingMessage) {
// Validate `Host` header
const hostHeader = req.headers.host;
if (hostHeader == null) return false;
try {
const host = new URL(`http://${hostHeader}`);
if (!ALLOWED_HOST_HOSTNAMES.includes(host.hostname)) return false;
} catch {
return false;
}
// Validate `Origin` header
let originHeader = req.headers.origin;
if (originHeader === null && req.headers["user-agent"] === undefined) {
// VSCode doesn't send an `Origin` header, but also doesn't send a
// `User-Agent` header, so allow an empty origin in this case.
originHeader = "http://localhost";
}
if (originHeader == null) return false;
try {
const origin = new URL(originHeader);
const allowed = ALLOWED_ORIGIN_HOSTNAMES.some((rule) => {
if (typeof rule === "string") return origin.hostname === rule;
else return rule.test(origin.hostname);
});
if (!allowed) return false;
} catch {
return false;
}
return true;
}

export default function useInspector(props: InspectorProps) {
/** A unique ID for this session. */
const inspectorIdRef = useRef(randomId());
Expand Down Expand Up @@ -110,7 +150,7 @@ export default function useInspector(props: InspectorProps) {
case "/json/list":
{
res.setHeader("Content-Type", "application/json");
const localHost = `localhost:${props.port}/ws`;
const localHost = `127.0.0.1:${props.port}/ws`;
const devtoolsFrontendUrl = `devtools://devtools/bundled/js_app.html?experiments=true&v8only=true&ws=${localHost}`;
const devtoolsFrontendUrlCompat = `devtools://devtools/bundled/inspector.html?experiments=true&v8only=true&ws=${localHost}`;
res.end(
Expand Down Expand Up @@ -155,7 +195,12 @@ export default function useInspector(props: InspectorProps) {
}
const wsServer = wsServerRef.current;

wsServer.on("connection", (ws: WebSocket) => {
wsServer.on("connection", (ws: WebSocket, req: IncomingMessage) => {
if (!validateWebSocketHandshake(req)) {
ws.close(1008, "Unauthorised");
return;
}

if (wsServer.clients.size > 1) {
/** We only want to have one active Devtools instance at a time. */
logger.error(
Expand Down Expand Up @@ -197,7 +242,7 @@ export default function useInspector(props: InspectorProps) {
timeout: 2000,
abortSignal: abortController.signal,
});
server.listen(props.port);
server.listen(props.port, "127.0.0.1");
}
startInspectorProxy().catch((err) => {
if ((err as { code: string }).code !== "ABORT_ERR") {
Expand Down Expand Up @@ -844,7 +889,7 @@ export const openInspector = async (
) => {
const query = new URLSearchParams();
query.set("theme", "systemPreferred");
query.set("ws", `localhost:${inspectorPort}/ws`);
query.set("ws", `127.0.0.1:${inspectorPort}/ws`);
if (worker) query.set("domain", worker);
const url = `https://devtools.devprod.cloudflare.dev/js_app?${query.toString()}`;
const errorMessage =
Expand Down
Loading