Bind 0.0.0.0 to IPv4 only (closes #528)

Go listens on any unspecified address with one socket that answers
IPv4 and IPv6, so BIND_ADDRESS=0.0.0.0 also bound every IPv6 address,
which the README says it does not. The listener now uses tcp4 for an
IPv4 address; :: still binds both. The server's start hook suppresses
contextcheck, as the other start hooks do.

The image's health check asked for localhost, which wget reached over
::1 only, so it now asks for 127.0.0.1. The README says what that
means for a loopback bind in a container, drops the bindv6only
condition from :: (Go sets that socket option itself), and names an
ss check.

Model: opus-5-5

Dockerfile

@@ -274,8 +274,11 @@ EXPOSE 8080
 # port.
 ENV BIND_ADDRESS=0.0.0.0
 
+# The address is 127.0.0.1 because wget tries only the first address
+# localhost resolves to. That can be ::1, which a 0.0.0.0 bind does
+# not answer.
 HEALTHCHECK --interval=30s --timeout=3s --start-period=5s --retries=3 \
-    CMD wget --no-verbose --tries=1 --spider http://localhost:8080/.well-known/healthcheck || exit 1
+    CMD wget --no-verbose --tries=1 --spider http://127.0.0.1:8080/.well-known/healthcheck || exit 1
 
 ENTRYPOINT ["/usr/local/bin/docker-entrypoint.sh"]
 CMD ["/app/webhooker"]

README.md

@@ -316,10 +316,9 @@ The value must be an IP address literal:
 
 - `127.0.0.1` — loopback only (the binary's default). Use this with a reverse
   proxy on the same host.
-- `0.0.0.0` — every IPv4 address. The image's default; on a bare host, only
+- `0.0.0.0` — every IPv4 address and no IPv6 address. The image's default; on a
-  behind a firewall on the port.
+  bare host, only behind a firewall on the port.
-- `::` — every address, IPv6 and (on Linux, with the default
+- `::` — every IPv6 address and every IPv4 address.
-  `net.ipv6.bindv6only=0`) IPv4 as well.
 - A specific address such as `10.0.0.5` — that interface only.
 
 An **empty** value is treated as unset, as everywhere else here, and takes the
@@ -340,7 +339,8 @@ machine that has no such interface — is a valid literal, so it reaches the
 listener and fails there. The process logs the bind error and exits non-zero
 rather than staying up with nothing listening. The effective value is in the
 `bindAddress` field of the startup log line, which is the way to check what a
-running deployment actually bound.
+running deployment actually bound. On a bare host, `ss -ltn` lists the socket
+itself as `0.0.0.0:8080` for a `0.0.0.0` bind and as `*:8080` for `::`.
 
 #### Metrics credentials
 
@@ -667,10 +667,10 @@ Either set a literal or leave the variable out entirely.
 
 Overriding `BIND_ADDRESS` to a loopback address in a container — by that route
 or deliberately — makes the container unreachable from outside its namespace
-even with `-p`. The published port answers nothing, and the health check fails
+even with `-p`. The published port answers nothing. The health check requests
-too: it requests `http://localhost:8080`, `localhost` resolves to `::1` first,
+`http://127.0.0.1:8080` from inside the container, so a `127.0.0.1` bind passes
-and a `127.0.0.1` bind is not listening there. The container then goes
+it and the container stays `healthy`. A `::1` bind fails it, and the container
-`unhealthy` about **65 seconds** after start — from
+goes `unhealthy` about **65 seconds** after start — from
 `HEALTHCHECK --start-period=5s --interval=30s --retries=3`, so failing probes at
 5s, 35s and 65s, and `unhealthy` on the third. (Docker's probe cadence during
 the start period has changed between versions; re-derive from those three values
@@ -726,7 +726,7 @@ the credentials.
       address (directly, or through anything that can rewrite source addresses,
       such as NAT or a published container port). The `remoteIP` field of the
       `http request` log line for a request that came through the proxy shows
-      that address; the health check's own lines show `::1`. See
+      that address; the health check's own lines show `127.0.0.1`. See
       [Trusted proxies](#trusted-proxies).
     - Leave `BIND_ADDRESS` and `DATA_DIR` unset: the image sets `BIND_ADDRESS`
       to `0.0.0.0`, and `DATA_DIR` defaults to `/var/lib/webhooker`.

internal/delivery/delivery_totals_test.go

@@ -32,9 +32,8 @@ func targetTotals(
 
 // TestUpdateDeliveryStatus_FinishTimeAndTargetTotals pins what a status
 // write records for the webhook page's statistics: the time a delivery
-// finished, set only when it becomes delivered or failed and stored in
+// finished, set only when it becomes delivered or failed, and one more
-// the same form as created_at, and one more on its target's delivered
+// on its target's delivered or failed total.
-// or failed total.
 func TestUpdateDeliveryStatus_FinishTimeAndTargetTotals(t *testing.T) {
 	t.Parallel()
 
@@ -76,16 +75,6 @@ func TestUpdateDeliveryStatus_FinishTimeAndTargetTotals(t *testing.T) {
 			if tt.finished {
 				require.NotNil(t, stored.FinishedAt)
 				assert.False(t, stored.FinishedAt.Before(before))
-
-				var createdAt, finishedAt string
-
-				require.NoError(t, db.Raw(
-					`SELECT CAST(created_at AS TEXT),
-						CAST(finished_at AS TEXT)
-					FROM deliveries WHERE id = ?`, d.ID,
-				).Row().Scan(&createdAt, &finishedAt))
-				assertStoredTime(t, createdAt)
-				assertStoredTime(t, finishedAt)
 			} else {
 				assert.Nil(t, stored.FinishedAt)
 			}

internal/delivery/engine.go

@@ -1701,11 +1701,8 @@ func writeDeliveryStatus(
 			database.DeliveryStatusFailed,
 		}).
 		Updates(map[string]any{
-			"status": status,
+			"status":      status,
-			// GORM's clock, the one that stamps created_at. A bare
+			"finished_at": time.Now(),
-			// time.Now() carries a monotonic clock reading, which the
-			// driver would store as an " m=+…" suffix.
-			"finished_at": tx.NowFunc(),
 		})
 	if res.Error != nil || res.RowsAffected == 0 {
 		return res.Error
@@ -1909,13 +1906,12 @@ func (e *Engine) takeForRedispatch(
 		return false
 	}
 
-	// GORM's clock, as writeDeliveryStatus explains.
 	res := webhookDB.
 		Model(&database.Delivery{}).
 		Where(
 			"id = ? AND status = ?", deliveryID, eligible,
 		).
-		UpdateColumn("updated_at", webhookDB.NowFunc())
+		UpdateColumn("updated_at", time.Now())
 
 	if res.Error != nil {
 		e.log.Error(

internal/delivery/engine_test.go

@@ -117,18 +117,6 @@ func seedDelivery(
 	return d
 }
 
-// assertStoredTime asserts that text, a timestamp as SQLite holds it,
-// is in the form GORM stores created_at in: the layout of
-// time.Time.String, with no " m=+…" monotonic clock reading after it.
-func assertStoredTime(t *testing.T, text string) {
-	t.Helper()
-
-	_, err := time.Parse(
-		"2006-01-02 15:04:05.999999999 -0700 MST", text,
-	)
-	assert.NoError(t, err, "stored as %q", text)
-}
-
 // httpDeliveryFixture holds the task and delivery needed
 // for HTTP delivery tests.
 type httpDeliveryFixture struct {

internal/delivery/inflight_test.go

@@ -462,8 +462,7 @@ func TestScheduledRetryIsNotSweptDuringBackoff(t *testing.T) {
 
 // TestRedispatchStampsTheRow pins the cadence control: a stranded
 // delivery that has just been handed out is not selected again by the
-// next tick a minute later. The stamp is stored in the same form as
+// next tick a minute later.
-// created_at.
 func TestRedispatchStampsTheRow(t *testing.T) {
 	t.Parallel()
 
@@ -494,13 +493,4 @@ func TestRedispatchStampsTheRow(t *testing.T) {
 		"a re-dispatched delivery must be stamped so the "+
 			"next tick does not select it again",
 	)
-
-	var createdAt, updatedAt string
-
-	require.NoError(t, s.WebhookDB.Raw(
-		`SELECT CAST(created_at AS TEXT), CAST(updated_at AS TEXT)
-		FROM deliveries WHERE id = ?`, d.ID,
-	).Row().Scan(&createdAt, &updatedAt))
-	assertStoredTime(t, createdAt)
-	assertStoredTime(t, updatedAt)
 }

internal/delivery/target_database_archive.go

@@ -277,11 +277,7 @@ func (w *archiveWriter) write(
 		}
 	}
 
-	// GORM's clock, the one that stamps created_at in the other
+	row.ArchivedAt = time.Now()
-	// databases, so this is stored in the same form. A bare time.Now()
-	// carries a monotonic clock reading, which the driver would store
-	// as an " m=+…" suffix.
-	row.ArchivedAt = w.db.NowFunc()
 
 	err := w.db.Create(&row).Error
 	if err != nil {

internal/delivery/target_database_test.go

@@ -150,13 +150,6 @@ func TestArchiveWriter_WritesRow(t *testing.T) {
 	assert.Equal(t, row.ContentType, got[0].ContentType)
 	assert.JSONEq(t, `{"hello":"world"}`, got[0].Body)
 	assert.False(t, got[0].ArchivedAt.IsZero())
-
-	var archivedAt string
-
-	require.NoError(t, w.DB().Raw(
-		"SELECT CAST(archived_at AS TEXT) FROM archived_events",
-	).Row().Scan(&archivedAt))
-	assertStoredTime(t, archivedAt)
 }
 
 func TestArchiveWriter_RecreatesAfterRemoval(

internal/server/bind_address_test.go

@@ -5,6 +5,7 @@ import (
 	"net"
 	"net/http"
 	"strconv"
+	"syscall"
 	"testing"
 	"time"
 
@@ -25,6 +26,11 @@ const (
 	// its network namespace even with a published port.
 	wildcardV4 = "0.0.0.0"
 
+	// loopbackV6 and wildcardV6 are the IPv6 counterparts of the two
+	// above.
+	loopbackV6 = "::1"
+	wildcardV6 = "::"
+
 	// unavailableAddr is a TEST-NET-1 address (RFC 5737). It is a
 	// well-formed literal that no host is assigned, so binding it
 	// fails with EADDRNOTAVAIL rather than succeeding somewhere
@@ -116,6 +122,24 @@ func otherLocalAddr(t *testing.T) string {
 	return ""
 }
 
+// skipWithoutIPv6 skips the test when this host cannot bind
+// loopbackV6. Without IPv6 there is no second family for a bind to
+// claim or leave alone.
+func skipWithoutIPv6(t *testing.T) {
+	t.Helper()
+
+	var listenCfg net.ListenConfig
+
+	l, err := listenCfg.Listen(
+		t.Context(), "tcp6", net.JoinHostPort(loopbackV6, "0"),
+	)
+	if err != nil {
+		t.Skip("host has no IPv6 loopback to bind")
+	}
+
+	require.NoError(t, l.Close())
+}
+
 // startBoundServer starts the wired app with the given bind address
 // on a free port and returns that port. The app is stopped on
 // cleanup.
@@ -292,6 +316,50 @@ func TestBindAddress_WildcardReachesOtherAddresses(t *testing.T) {
 	requireDialable(t, net.JoinHostPort(other, strconv.Itoa(port)))
 }
 
+// TestBindAddress_WildcardV4IsIPv4Only pins that 0.0.0.0 answers on
+// IPv4 alone, as the README says. Go listens on 0.0.0.0 with a socket
+// that answers IPv6 too unless told otherwise, which put the admin UI
+// and the receiver on every IPv6 address of a host whose operator had
+// chosen an IPv4 bind.
+func TestBindAddress_WildcardV4IsIPv4Only(t *testing.T) {
+	t.Parallel()
+
+	skipWithoutIPv6(t)
+
+	port := startBoundServer(t, wildcardV4)
+
+	// Positive control: the service really is up, on IPv4.
+	requireDialable(t, net.JoinHostPort(loopbackV4, strconv.Itoa(port)))
+
+	dialer := net.Dialer{Timeout: dialTimeout}
+
+	conn, err := dialer.DialContext(
+		t.Context(), "tcp",
+		net.JoinHostPort(loopbackV6, strconv.Itoa(port)),
+	)
+	if err == nil {
+		_ = conn.Close()
+	}
+
+	require.ErrorIs(
+		t, err, syscall.ECONNREFUSED,
+		"a %s bind answered on %s", wildcardV4, loopbackV6,
+	)
+}
+
+// TestBindAddress_WildcardV6ReachesBothFamilies is the counterpart:
+// :: is the value that binds IPv6, and it answers IPv4 as well.
+func TestBindAddress_WildcardV6ReachesBothFamilies(t *testing.T) {
+	t.Parallel()
+
+	skipWithoutIPv6(t)
+
+	port := startBoundServer(t, wildcardV6)
+
+	requireDialable(t, net.JoinHostPort(loopbackV6, strconv.Itoa(port)))
+	requireDialable(t, net.JoinHostPort(loopbackV4, strconv.Itoa(port)))
+}
+
 // TestBindAddress_ServesRequestsOnConfiguredAddress proves the bound
 // listener serves the application rather than merely accepting TCP,
 // so a bind address that is honoured cannot be mistaken for one that

internal/server/http.go

@@ -1,9 +1,11 @@
 package server
 
 import (
+	"context"
 	"errors"
 	"net"
 	"net/http"
+	"net/netip"
 	"strconv"
 	"time"
 )
@@ -39,6 +41,21 @@ func (s *Server) listenAddr() string {
 	)
 }
 
+// listenNetwork returns the network the HTTP listener binds on, "tcp4"
+// for an IPv4 BindAddress and "tcp" for any other.
+//
+// On "tcp", Go binds 0.0.0.0 with a socket that answers IPv6 as well,
+// exactly as it binds ::. "tcp4" keeps 0.0.0.0 to IPv4, which is what
+// the README says it means. :: stays on "tcp" and answers both.
+func (s *Server) listenNetwork() string {
+	addr, err := netip.ParseAddr(s.params.Config.BindAddress)
+	if err == nil && addr.Is4() {
+		return "tcp4"
+	}
+
+	return "tcp"
+}
+
 // newHTTPServer builds the HTTP server for this Server's
 // configuration.
 //
@@ -67,7 +84,17 @@ func (s *Server) serveUntilShutdown() {
 
 	s.log.Info("http begin listen", "listenaddr", s.httpServer.Addr)
 
-	err := s.httpServer.ListenAndServe()
+	// ListenAndServe always listens on "tcp", which would undo
+	// listenNetwork.
+	var listenCfg net.ListenConfig
+
+	listener, err := listenCfg.Listen(
+		context.Background(), s.listenNetwork(), s.httpServer.Addr,
+	)
+	if err == nil {
+		err = s.httpServer.Serve(listener)
+	}
+
 	if err != nil && !errors.Is(err, http.ErrServerClosed) {
 		s.log.Error("listen error", "error", err)
 		s.shutdownWithFailure()

internal/server/server.go

@@ -135,6 +135,8 @@ func New(lc fx.Lifecycle, params ServerParams) (*Server, error) {
 	s.httpServer = s.newHTTPServer()
 
 	lc.Append(fx.Hook{
+		//nolint:contextcheck // The listener outlives this hook, whose
+		// context fx cancels once the app has started.
 		OnStart: func(_ context.Context) error {
 			s.startupTime = time.Now()
 			go s.Run()