Say how to allow a refused private target address (closes #398)

Adding or editing an http or slack target whose address is private or
reserved was refused with no hint that the refusal is deliberate or
that it can be lifted. The refusal now adds that such addresses are
refused by default and that the server's ALLOWED_EGRESS_CIDRS setting
allows named networks, naming the README section "Allowing egress to
your own network". Link-local and cloud metadata refusals, which no
setting can lift, are unchanged.

The delivery package's blocked-address error is exported so the
handler can tell this refusal from the others.

Model: opus-5-5

internal/delivery/ssrf.go

@@ -25,7 +25,9 @@ var (
 	errNoIPs      = errors.New(
 		"hostname resolved to no IP addresses",
 	)
-	errBlockedIP = errors.New(
+	// ErrBlockedIP reports an address the default blocklist
+	// refuses, one that ALLOWED_EGRESS_CIDRS can open.
+	ErrBlockedIP = errors.New(
 		"blocked private, reserved or cloud metadata address",
 	)
 	errBlockedMetadata = errors.New(
@@ -340,7 +342,7 @@ func (g *Guard) checkIP(ip net.IP) error {
 
 	if isBlockedIP(ip) {
 		return fmt.Errorf(
-			"target IP %s: %w", ip, errBlockedIP,
+			"target IP %s: %w", ip, ErrBlockedIP,
 		)
 	}
 

internal/handlers/source_management.go

@@ -1570,11 +1570,20 @@ func (h *Handlers) validateTargetURL(
 			"url", delivery.MaskURL(targetURL),
 			"error", err,
 		)
-		http.Error(
+
-			w,
+		msg := "Invalid target URL: " + err.Error()
-			"Invalid target URL: "+err.Error(),
+
-			http.StatusBadRequest,
+		// Only this refusal can be lifted by configuration, so
-		)
+		// only it says how. Link-local and metadata addresses
+		// stay refused whatever is configured.
+		if errors.Is(err, delivery.ErrBlockedIP) {
+			msg += ". Private and reserved addresses are refused " +
+				"by default; the server's ALLOWED_EGRESS_CIDRS " +
+				"setting allows named networks (see \"Allowing " +
+				"egress to your own network\" in the README)."
+		}
+
+		http.Error(w, msg, http.StatusBadRequest)
 
 		return err
 	}

internal/handlers/target_private_refusal_test.go

@@ -0,0 +1,98 @@
+package handlers_test
+
+import (
+	"net/http"
+	"net/url"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"sneak.berlin/go/webhooker/internal/database"
+)
+
+// privateRefusalHint is the sentence that tells an operator a private
+// destination is refused on purpose, and how to allow one.
+const privateRefusalHint = "Private and reserved addresses are " +
+	"refused by default; the server's ALLOWED_EGRESS_CIDRS setting " +
+	"allows named networks (see \"Allowing egress to your own " +
+	"network\" in the README)."
+
+// TestTargetRefusal_PrivateDestinationSaysHowToAllowIt covers both
+// target types that take a URL, on add and on edit.
+func TestTargetRefusal_PrivateDestinationSaysHowToAllowIt(
+	t *testing.T,
+) {
+	t.Parallel()
+
+	env := setupSourceTest(t)
+
+	targetTypes := []database.TargetType{
+		database.TargetTypeHTTP,
+		database.TargetTypeSlack,
+	}
+
+	for _, targetType := range targetTypes {
+		t.Run(string(targetType), func(t *testing.T) {
+			t.Parallel()
+
+			webhook := seedWebhookWithRetention(t, env.db, 30)
+			targetsPath := "/source/" + webhook.ID + "/targets"
+
+			form := url.Values{}
+			form.Set("name", "private")
+			form.Set("type", string(targetType))
+			form.Set("url", editBlockedURL)
+
+			added := serveTarget(
+				env, http.MethodPost, targetsPath, form,
+			)
+			assert.Equal(t, http.StatusBadRequest, added.Code)
+			assert.Contains(
+				t, added.Body.String(), privateRefusalHint,
+			)
+
+			form.Set("url", editOriginalURL)
+
+			created := serveTarget(
+				env, http.MethodPost, targetsPath, form,
+			)
+			require.Equal(
+				t, http.StatusSeeOther, created.Code,
+				created.Body.String(),
+			)
+
+			targets := targetsForWebhook(t, env.db, webhook.ID)
+			require.Len(t, targets, 1)
+
+			form.Set("url", editBlockedURL)
+
+			edited := submitTargetEdit(
+				env, webhook.ID, targets[0].ID, form,
+			)
+			assert.Equal(t, http.StatusBadRequest, edited.Code)
+			assert.Contains(
+				t, edited.Body.String(), privateRefusalHint,
+			)
+		})
+	}
+
+	// No setting opens a link-local address, so its refusal must
+	// not point at one.
+	t.Run("link-local", func(t *testing.T) {
+		t.Parallel()
+
+		webhook := seedWebhookWithRetention(t, env.db, 30)
+
+		form := url.Values{}
+		form.Set("name", "metadata")
+		form.Set("type", string(database.TargetTypeHTTP))
+		form.Set("url", "http://169.254.169.254/latest/meta-data/")
+
+		w := serveTarget(
+			env, http.MethodPost,
+			"/source/"+webhook.ID+"/targets", form,
+		)
+		assert.Equal(t, http.StatusBadRequest, w.Code)
+		assert.NotContains(t, w.Body.String(), privateRefusalHint)
+	})
+}