Skip to content

Commit

Permalink
[wrangler] fix: listen on loopback for wrangler dev port check and lo…
Browse files Browse the repository at this point in the history
…gin (#4830)

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
  • Loading branch information
Lekensteyn authored Feb 1, 2024
1 parent 37141ba commit 48f9085
Show file tree
Hide file tree
Showing 5 changed files with 36 additions and 14 deletions.
10 changes: 10 additions & 0 deletions .changeset/smart-owls-jog.md
Original file line number Diff line number Diff line change
@@ -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.
12 changes: 7 additions & 5 deletions packages/wrangler/src/__tests__/dev.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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(`""`);
Expand All @@ -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(`""`);
Expand Down
16 changes: 11 additions & 5 deletions packages/wrangler/src/dev.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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;
};
}
/**
Expand Down Expand Up @@ -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(
Expand Down
10 changes: 7 additions & 3 deletions packages/wrangler/src/dev/proxy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ export async function startPreviewServer({
accessTokenRef,
});

await waitForPortToBeAvailable(port, {
await waitForPortToBeAvailable(port, ip, {
retryPeriod: 200,
timeout: 2000,
abortSignal: abortController.signal,
Expand Down Expand Up @@ -295,7 +295,7 @@ export function usePreviewServer({
return;
}

waitForPortToBeAvailable(port, {
waitForPortToBeAvailable(port, ip, {
retryPeriod: 200,
timeout: 2000,
abortSignal: abortController.signal,
Expand Down Expand Up @@ -636,9 +636,13 @@ function createStreamHandler(
*/
export async function waitForPortToBeAvailable(
port: number,
host: string,
options: { retryPeriod: number; timeout: number; abortSignal: AbortSignal }
): Promise<void> {
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");
Expand Down Expand Up @@ -686,7 +690,7 @@ export async function waitForPortToBeAvailable(
doReject(err);
}
});
server.listen(port, () =>
server.listen(port, host, () =>
terminator
.terminate()
.then(doResolve, () =>
Expand Down
2 changes: 1 addition & 1 deletion packages/wrangler/src/user/user.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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}`);
Expand Down

0 comments on commit 48f9085

Please sign in to comment.