Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

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

next dev default listens on all interfaces #20137

Closed
kcking opened this issue Dec 13, 2020 · 8 comments · Fixed by #20409
Closed

next dev default listens on all interfaces #20137

kcking opened this issue Dec 13, 2020 · 8 comments · Fixed by #20409
Labels
Developer Experience Issues related to Next.js logs, Error overlay, etc.

Comments

@kcking
Copy link

kcking commented Dec 13, 2020

Bug report

Describe the bug

next dev command listens on all interfaces by default

To Reproduce

Steps to reproduce the behavior, please provide code snippets or a repository:

  1. run any next.js project with next dev
  2. run sudo lsof -nP -iTCP:3000 to see all listeners for port 3000, outputs
node    ...  TCP *:3000 (LISTEN)

Expected behavior

Should only listen on localhost and output

node    ... TCP 127.0.0.1:3000 (LISTEN)

Screenshots

I was first made aware of this via this tweet: https://twitter.com/rsms/status/1337845477283221504

System information

  • OS: macOS + WSL tested
  • Version of Next.js: 10.0.3
  • Version of Node.js: 12.19.0
  • Deployment: next dev
@kcking kcking added the bug Issue was opened via the bug report template. label Dec 13, 2020
@Flaque
Copy link

Flaque commented Dec 13, 2020

This is happening because it's the default behavior of Node if a host isn't passed in:

function startServer(
  serverOptions: any,
  port?: number,
  hostname?: string
) {  
  // ...
  srv.listen(port, hostname)
  // ...
}

start-server.ts#L18

In the next/cli/next-dev.ts, it's passed in via a --hostname argument if you want though:

startServer(
    { dir, dev: true, isNextDevCommand: true },
    port,
    args['--hostname']
) { /* ... */

next-dev.ts#L111-L115

@kcking & @rsms, I don't know enough about this yet. Is there a case in a development environment where it would be an issue to listen on all hosts? Conversely, are there common tools in development environments that may have caused node to make this the default?

@rsms
Copy link

rsms commented Dec 13, 2020

@Flaque Is there a case in a development environment where it would be an issue to listen on all hosts?

If the user is not careful to run a firewall they would unknowingly open up a door for the internet (i.e. anyone) to access their server. I'd strongly suggest that the default is to allow only connections from the local machine (i.e. listen on/bind to localhost) and make "allow connections from the internet" an option that must be explicitly provided. This approach is commonplace in all software-development teams I've worked with as it also reduces issues from a larger LAN that might share a firewall.

If you for some reason really want the default to be "allow connections from the internet", you probably want to make sure that the message presented to the user reflects that. For example listening on http://0.0.0.0:8000 (or http://[::]:8000 if the connection is in IPv6 mode.)

@timneutkens
Copy link
Member

timneutkens commented Dec 17, 2020

I'm fine with changing the default host to 127.0.0.1 however one concern I have with making this change is that it'll break certain development setups that currently work meaning we might have to release this change in a major version. What are your thoughts on this?

@timneutkens timneutkens added kind: story and removed bug Issue was opened via the bug report template. labels Dec 17, 2020
@timneutkens timneutkens added this to the 10.x.x milestone Dec 17, 2020
@rsms
Copy link

rsms commented Dec 17, 2020

I'm fine with changing the default host to 127.0.0.1 however one concern I have with making this change is that it'll break certain development setups that currently work meaning we might have to release this change in a major version. What are your thoughts on this?

That's a good point and I think very valid concern. How about changing the log message to contain the actual hostname? It seems it does reflect the correct port already so chances are there is code already to customize that log message. Another thing you might want to consider is to output an additional message when binding to "any", something like "Open publicly to the internet."

Some more thoughts on safety: In serve-http, a little program I use to do web development locally, I make sure to avoid serving possibly-sensitive parts of the file system when the server is accepting outside connections. This was inspired by Evan W's servedir that is used internally at Figma.

When you run serve-http accepting public connections, it screams a little at you:

$ npm install -g serve-http
$ serve-http -p 8090 -public
serving ./ PUBLICLY TO THE INTERNET at http://192.168.7.41:8090/ (livereload on :18090)
^C
$ serve-http -p 8090
serving ./ at http://localhost:8090/ (livereload on :18090)

@chulkilee
Copy link
Contributor

This happens on next start as well. When hostname option is not give, it binds to 0.0.0.0, not 127.0.0.1

Note that changing the default from 0.0.0.0 to 127.0.0.1 even in dev would break existing flow, such as docker-based workflow.

For now Next.js MUST fix the message at least, when hostname option is not given (both start and dev mode). Created #20409 for that

@denis-sokolov
Copy link

@timneutkens, perhaps you did not intend the merged PR to close this issue? It is my understanding the issue is about having an arguably insecure default and that has not been changed. Or do you feel like the decision to keep an insecure default for backwards compatibility has been made?

Personally, I think the backwards compatibility concerns can be alleviated, but even if the decision will not be changed, the console output can be more explicit about the insecure setting. “started server on 0.0.0.0:3000, url://localhost:3000” between other console messages is quite a subtle way to communicate that source code for the project is fully available to possible adversaries on the network.

@timneutkens timneutkens reopened this May 26, 2021
@timneutkens
Copy link
Member

This one auto-closed, we'll want to change the default eventually 👍

@timneutkens timneutkens removed this from the iteration 16 milestone Nov 17, 2021
@timneutkens timneutkens added the Developer Experience Issues related to Next.js logs, Error overlay, etc. label Nov 18, 2021
@jankaifer jankaifer self-assigned this Dec 1, 2022
@jankaifer jankaifer added the please verify canary The issue should be verified against next@canary. It will be closed after 30 days of inactivity label Dec 1, 2022
@vercel vercel deleted a comment from github-actions bot Dec 16, 2022
@timneutkens timneutkens removed the please verify canary The issue should be verified against next@canary. It will be closed after 30 days of inactivity label Dec 16, 2022
@fab1an
Copy link

fab1an commented Jul 23, 2023

Another issue here is that the new child processes like "next-router-worker" are listening on all interfaces EVEN if you start next.js with hostname=127.0.0.1.

@vercel vercel locked and limited conversation to collaborators Apr 17, 2024
@balazsorban44 balazsorban44 converted this issue into discussion #64650 Apr 17, 2024

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Labels
Developer Experience Issues related to Next.js logs, Error overlay, etc.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants