From 48f9085981f0a4923d3ccc32596520107c4e4df8 Mon Sep 17 00:00:00 2001 From: Peter Wu Date: Thu, 1 Feb 2024 11:35:29 +0100 Subject: [PATCH] [wrangler] fix: listen on loopback for wrangler dev port check and login (#4830) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `wrangler dev` port availability check triggers a firewall prompt on macOS while it briefly opens and closes listeners. The OAuth callback server from `wrangler login` has the same issue. Fix both cases by listening on the loopback address only by default. Fixed some new test failures by using locally available IP addresses: wrangler:test: ● wrangler dev › ip › should use to `ip` from `wrangler.toml`, if available wrangler:test: listen EADDRNOTAVAIL: address not available 1.2.3.4:8787 wrangler:test: wrangler:test: ● wrangler dev › ip › should use --ip command line arg, if provided wrangler:test: listen EADDRNOTAVAIL: address not available 5.6.7.8:8787 Relates to #4430 --- .changeset/smart-owls-jog.md | 10 ++++++++++ packages/wrangler/src/__tests__/dev.test.tsx | 12 +++++++----- packages/wrangler/src/dev.tsx | 16 +++++++++++----- packages/wrangler/src/dev/proxy.ts | 10 +++++++--- packages/wrangler/src/user/user.ts | 2 +- 5 files changed, 36 insertions(+), 14 deletions(-) create mode 100644 .changeset/smart-owls-jog.md diff --git a/.changeset/smart-owls-jog.md b/.changeset/smart-owls-jog.md new file mode 100644 index 000000000000..d987f63cc23c --- /dev/null +++ b/.changeset/smart-owls-jog.md @@ -0,0 +1,10 @@ +--- +"wrangler": patch +--- + +fix: listen on loopback for wrangler dev port check and login + +Avoid listening on the wildcard address by default to reduce the attacker's +surface and avoid firewall prompts on macOS. + +Relates to #4430. diff --git a/packages/wrangler/src/__tests__/dev.test.tsx b/packages/wrangler/src/__tests__/dev.test.tsx index 66f5d4580942..8b5d4c459a4e 100644 --- a/packages/wrangler/src/__tests__/dev.test.tsx +++ b/packages/wrangler/src/__tests__/dev.test.tsx @@ -879,12 +879,12 @@ describe("wrangler dev", () => { writeWranglerToml({ main: "index.js", dev: { - ip: "1.2.3.4", + ip: "::1", }, }); fs.writeFileSync("index.js", `export default {};`); await runWrangler("dev"); - expect((Dev as jest.Mock).mock.calls[0][0].initialIp).toEqual("1.2.3.4"); + expect((Dev as jest.Mock).mock.calls[0][0].initialIp).toEqual("::1"); expect(std.out).toMatchInlineSnapshot(`""`); expect(std.warn).toMatchInlineSnapshot(`""`); expect(std.err).toMatchInlineSnapshot(`""`); @@ -894,12 +894,14 @@ describe("wrangler dev", () => { writeWranglerToml({ main: "index.js", dev: { - ip: "1.2.3.4", + ip: "::1", }, }); fs.writeFileSync("index.js", `export default {};`); - await runWrangler("dev --ip=5.6.7.8"); - expect((Dev as jest.Mock).mock.calls[0][0].initialIp).toEqual("5.6.7.8"); + await runWrangler("dev --ip=127.0.0.1"); + expect((Dev as jest.Mock).mock.calls[0][0].initialIp).toEqual( + "127.0.0.1" + ); expect(std.out).toMatchInlineSnapshot(`""`); expect(std.warn).toMatchInlineSnapshot(`""`); expect(std.err).toMatchInlineSnapshot(`""`); diff --git a/packages/wrangler/src/dev.tsx b/packages/wrangler/src/dev.tsx index b6f4859d370f..c0bd22261619 100644 --- a/packages/wrangler/src/dev.tsx +++ b/packages/wrangler/src/dev.tsx @@ -626,12 +626,16 @@ export async function startApiDev(args: StartDevOptions) { }; } /** + * Get an available TCP port number. + * * Avoiding calling `getPort()` multiple times by memoizing the first result. */ -function memoizeGetPort(defaultPort?: number) { +function memoizeGetPort(defaultPort: number, host: string) { let portValue: number; return async () => { - return portValue || (portValue = await getPort({ port: defaultPort })); + // Check a specific host to avoid probing all local addresses. + portValue = portValue ?? (await getPort({ port: defaultPort, host: host })); + return portValue; }; } /** @@ -705,14 +709,16 @@ async function validateDevServerSettings( ); const { zoneId, host, routes } = await getZoneIdHostAndRoutes(args, config); - const getLocalPort = memoizeGetPort(DEFAULT_LOCAL_PORT); - const getInspectorPort = memoizeGetPort(DEFAULT_INSPECTOR_PORT); + const initialIp = args.ip || config.dev.ip; + const initialIpListenCheck = initialIp === "*" ? "0.0.0.0" : initialIp; + const getLocalPort = memoizeGetPort(DEFAULT_LOCAL_PORT, initialIpListenCheck); + const getInspectorPort = memoizeGetPort(DEFAULT_INSPECTOR_PORT, "localhost"); // Our inspector proxy server will be binding to the result of // `getInspectorPort`. If we attempted to bind workerd to the same inspector // port, we'd get a port already in use error. Therefore, generate a new port // for our runtime to bind its inspector service to. - const getRuntimeInspectorPort = memoizeGetPort(); + const getRuntimeInspectorPort = memoizeGetPort(0, "localhost"); if (config.services && config.services.length > 0) { logger.warn( diff --git a/packages/wrangler/src/dev/proxy.ts b/packages/wrangler/src/dev/proxy.ts index fe9fa3f8c633..ad3207e23c6d 100644 --- a/packages/wrangler/src/dev/proxy.ts +++ b/packages/wrangler/src/dev/proxy.ts @@ -154,7 +154,7 @@ export async function startPreviewServer({ accessTokenRef, }); - await waitForPortToBeAvailable(port, { + await waitForPortToBeAvailable(port, ip, { retryPeriod: 200, timeout: 2000, abortSignal: abortController.signal, @@ -295,7 +295,7 @@ export function usePreviewServer({ return; } - waitForPortToBeAvailable(port, { + waitForPortToBeAvailable(port, ip, { retryPeriod: 200, timeout: 2000, abortSignal: abortController.signal, @@ -636,9 +636,13 @@ function createStreamHandler( */ export async function waitForPortToBeAvailable( port: number, + host: string, options: { retryPeriod: number; timeout: number; abortSignal: AbortSignal } ): Promise { return new Promise((resolve, reject) => { + if (host === "*") { + host = "0.0.0.0"; + } // eslint-disable-next-line @typescript-eslint/no-explicit-any options.abortSignal.addEventListener("abort", () => { const abortError = new Error("waitForPortToBeAvailable() aborted"); @@ -686,7 +690,7 @@ export async function waitForPortToBeAvailable( doReject(err); } }); - server.listen(port, () => + server.listen(port, host, () => terminator .terminate() .then(doResolve, () => diff --git a/packages/wrangler/src/user/user.ts b/packages/wrangler/src/user/user.ts index 90cc3a2fbbf2..f03ce3dbdc41 100644 --- a/packages/wrangler/src/user/user.ts +++ b/packages/wrangler/src/user/user.ts @@ -1014,7 +1014,7 @@ export async function login( } }); - server.listen(8976); + server.listen(8976, "localhost"); }); if (props?.browser) { logger.log(`Opening a link in your default browser: ${urlToOpen}`);