Bind the app port deliberately and document the proxy deployment (closes #268)
All checks were successful
check / check (push) Successful in 3m1s
All checks were successful
check / check (push) Successful in 3m1s
The plaintext listener bound `:PORT`, so it answered on every interface with no way to say otherwise. That published the admin UI and the unauthenticated receiver in cleartext beside whatever TLS proxy was in front of them, reachable from any host that could route to the machine. BIND_ADDRESS now selects the address, defaulting to 127.0.0.1. Loopback is the only default that does not silently expose that listener; reaching webhooker from another host becomes a deliberate act. A container must set BIND_ADDRESS=0.0.0.0, since a loopback bind inside a network namespace is unreachable even with -p, and the documented `docker run` does. Only IP address literals are accepted: hostnames, host:port and CIDR blocks abort startup naming the variable and the value, and a literal that is not an address of this host fails at listen and exits non-zero. The http.Server is now built in New rather than in the serving goroutine. Two goroutines reached that field with nothing ordering them — the serving goroutine assigning it, the fx stop hook calling Shutdown on it — which is a data race the race detector reports as soon as anything starts and stops the server, and a nil dereference if a stop arrived first. README gains a "Deployment behind a reverse proxy" section: a working nginx server block, and the five things that are silent when wrong — bind or firewall the app port, WEBHOOKER_ENVIRONMENT=prod, TRUSTED_PROXIES, Host as $http_host rather than $host, and keeping the proxy's access log because webhooker's own records only the proxy.
This commit is contained in:
@@ -2,8 +2,9 @@ package server
|
||||
|
||||
import (
|
||||
"errors"
|
||||
"fmt"
|
||||
"net"
|
||||
"net/http"
|
||||
"strconv"
|
||||
"time"
|
||||
)
|
||||
|
||||
@@ -24,21 +25,47 @@ const (
|
||||
httpMaxHeaderBytes = 1 << 20
|
||||
)
|
||||
|
||||
func (s *Server) serveUntilShutdown() {
|
||||
listenAddr := fmt.Sprintf(":%d", s.params.Config.Port)
|
||||
s.httpServer = &http.Server{
|
||||
Addr: listenAddr,
|
||||
// listenAddr renders the address the HTTP listener binds.
|
||||
//
|
||||
// The host half is always present: an empty host would be the
|
||||
// wildcard, and the whole point of BIND_ADDRESS is that binding every
|
||||
// interface is a choice the operator makes rather than one the
|
||||
// process makes for them. Config guarantees a literal, so
|
||||
// JoinHostPort's bracketing is enough to make IPv6 well formed.
|
||||
func (s *Server) listenAddr() string {
|
||||
return net.JoinHostPort(
|
||||
s.params.Config.BindAddress,
|
||||
strconv.Itoa(s.params.Config.Port),
|
||||
)
|
||||
}
|
||||
|
||||
// newHTTPServer builds the HTTP server for this Server's
|
||||
// configuration.
|
||||
//
|
||||
// It is called from New, on the constructing goroutine, rather than
|
||||
// from the serving goroutine that used to assign s.httpServer
|
||||
// directly. Two goroutines reach that field — the serving goroutine
|
||||
// and the fx stop hook, which calls Shutdown on it — with nothing
|
||||
// ordering them. Constructing it during New puts the write before
|
||||
// every hook fx will later run, which both removes the race and rules
|
||||
// out the nil dereference a stop that arrived before the serving
|
||||
// goroutine had run would have caused.
|
||||
func (s *Server) newHTTPServer() *http.Server {
|
||||
return &http.Server{
|
||||
Addr: s.listenAddr(),
|
||||
ReadTimeout: httpReadTimeout,
|
||||
WriteTimeout: httpWriteTimeout,
|
||||
MaxHeaderBytes: httpMaxHeaderBytes,
|
||||
Handler: s,
|
||||
}
|
||||
}
|
||||
|
||||
func (s *Server) serveUntilShutdown() {
|
||||
// add routes
|
||||
// this does any necessary setup in each handler
|
||||
s.SetupRoutes()
|
||||
|
||||
s.log.Info("http begin listen", "listenaddr", listenAddr)
|
||||
s.log.Info("http begin listen", "listenaddr", s.httpServer.Addr)
|
||||
|
||||
err := s.httpServer.ListenAndServe()
|
||||
if err != nil && !errors.Is(err, http.ErrServerClosed) {
|
||||
|
||||
Reference in New Issue
Block a user