Mask the http target's destination URL in the UI (closes #115)

An http target's destination is frequently a Slack, Discord or
Teams incoming-webhook endpoint whose path segments are the
credential — the same property that made the Slack target's
webhook URL a bearer token. The source detail page rendered it
in full, so the leak closed for slack targets stayed reachable
through a different target type.

Render it through the existing MaskURL, which reduces a URL to
scheme and host. The field accepts an arbitrary URL, so no path
segment can be assumed non-secret and none is shown.

internal/delivery/target_config_view.go

@@ -106,6 +106,12 @@ func slackConfigFields(configJSON string) []ConfigField {
 // and its retry settings. Header values are not shown — they
 // routinely carry authorization tokens — only how many are
 // configured.
+//
+// The destination is masked to scheme and host by the same
+// rule the Slack target uses. An HTTP target's destination is
+// commonly a Slack, Discord or Teams incoming-webhook endpoint
+// whose path segments are the credential, and the field takes
+// an arbitrary URL, so no segment can be assumed non-secret.
 func httpConfigFields(t *database.Target) []ConfigField {
 	cfg, err := parseHTTPConfig(t.Config)
 	if err != nil {
@@ -114,7 +120,7 @@ func httpConfigFields(t *database.Target) []ConfigField {
 
 	fields := []ConfigField{{
 		Label: "Destination URL",
-		Value: cfg.URL,
+		Value: MaskURL(cfg.URL),
 	}}
 
 	if cfg.Timeout > 0 {

internal/delivery/target_config_view_test.go

@@ -19,6 +19,7 @@ const (
 
 	viewExampleOrigin = "https://example.com"
 	viewExampleHook   = viewExampleOrigin + "/hook"
+	viewMaskedOrigin  = viewExampleOrigin + "/..."
 	viewUnavailable   = "(unavailable)"
 	viewExpiryNever   = "never"
 )
@@ -162,7 +163,7 @@ func TestNewTargetViews_HTTP(t *testing.T) {
 	assert.Equal(
 		t,
 		map[string]string{
-			"Destination URL": viewExampleHook,
+			"Destination URL": viewMaskedOrigin,
 			"Timeout":         "30s",
 			"Headers":         "1 configured",
 			"Max Retries":     "5",
@@ -188,13 +189,41 @@ func TestNewTargetViews_HTTPFireAndForget(t *testing.T) {
 	assert.Equal(
 		t,
 		map[string]string{
-			"Destination URL": viewExampleHook,
+			"Destination URL": viewMaskedOrigin,
 			"Max Retries":     "0 (fire-and-forget)",
 		},
 		fieldMap(view.Config),
 	)
 }
 
+// TestNewTargetViews_HTTPMasksDestinationURL proves the rule
+// holds for the http target too: an http destination is
+// routinely an incoming-webhook endpoint whose path segments
+// are the credential, so none of them is shown.
+func TestNewTargetViews_HTTPMasksDestinationURL(t *testing.T) {
+	t.Parallel()
+
+	view := viewFor(t, database.Target{
+		Type:   database.TargetTypeHTTP,
+		Config: `{"url":"` + slackWebhookURL + `"}`,
+	})
+
+	fields := fieldMap(view.Config)
+
+	assert.Equal(
+		t,
+		"https://hooks.slack.com/...",
+		fields["Destination URL"],
+	)
+
+	for _, v := range fields {
+		assert.NotContains(t, v, slackSecretPath)
+		assert.NotContains(t, v, "T00000000")
+		assert.NotContains(t, v, "B00000000")
+		assert.NotContains(t, v, "XXXXXXXXXXXXXXXXXXXXXXXX")
+	}
+}
+
 func TestNewTargetViews_Database(t *testing.T) {
 	t.Parallel()
 

internal/handlers/export_test.go

@@ -1,19 +1,6 @@
 package handlers
 
-import (
+import "net/http"
-	"html/template"
-	"net/http"
-)
-
-// AddTemplateForTest registers a template under a page name so that
-// the handlers_test package can drive the render path with a
-// template of its own.
-func (s *Handlers) AddTemplateForTest(
-	pageTemplate string,
-	tmpl *template.Template,
-) {
-	s.templates[pageTemplate] = tmpl
-}
 
 // RenderTemplateForTest exposes renderTemplate for use in the
 // handlers_test package.

internal/handlers/handlers.go

@@ -3,7 +3,6 @@
 package handlers
 
 import (
-	"bytes"
 	"context"
 	"encoding/json"
 	"errors"
@@ -225,20 +224,13 @@ func (s *Handlers) renderTemplate(
 	s.executeTemplate(w, tmpl, wrapper)
 }
 
-// executeTemplate renders the template into a buffer and writes to
+// executeTemplate runs the template and handles errors.
-// the response only once rendering has fully succeeded. Executing
-// straight into the ResponseWriter commits a partial body and a 200
-// status before a mid-render error can be reported, leaving no way
-// to serve a 500. These pages are small, so holding one in memory is
-// the right trade.
 func (s *Handlers) executeTemplate(
 	w http.ResponseWriter,
 	tmpl *template.Template,
 	data any,
 ) {
-	var buf bytes.Buffer
+	err := tmpl.Execute(w, data)
-
-	err := tmpl.Execute(&buf, data)
 	if err != nil {
 		s.log.Error(
 			"failed to execute template", "error", err,
@@ -247,16 +239,5 @@ func (s *Handlers) executeTemplate(
 			w, "Internal server error",
 			http.StatusInternalServerError,
 		)
-
-		return
-	}
-
-	w.Header().Set("Content-Type", "text/html; charset=utf-8")
-
-	_, err = buf.WriteTo(w)
-	if err != nil {
-		s.log.Error(
-			"failed to write rendered page", "error", err,
-		)
 	}
 }

internal/handlers/handlers_test.go

@@ -2,8 +2,6 @@ package handlers_test
 
 import (
 	"context"
-	"errors"
-	"html/template"
 	"net/http"
 	"net/http/httptest"
 	"sync"
@@ -222,68 +220,6 @@ func TestRenderTemplate(t *testing.T) {
 	)
 }
 
-// errMidRender is the failure a test template raises partway through
-// rendering.
-var errMidRender = errors.New("deliberate mid-render failure")
-
-// midRenderFailure is template data whose first method renders and
-// whose second fails, so the template aborts after output has
-// already been produced.
-type midRenderFailure struct{}
-
-// Prefix is the output a streaming renderer would flush before the
-// failure below aborts the template.
-func (midRenderFailure) Prefix() string { return partialPageMarker }
-
-// Boom aborts template execution.
-func (midRenderFailure) Boom() (string, error) {
-	return "", errMidRender
-}
-
-// partialPageMarker is content the failing template emits before it
-// aborts.
-const partialPageMarker = "PARTIAL PAGE CONTENT"
-
-// TestRenderTemplateMidRenderErrorSendsNoPartialBody proves the
-// renderer does not commit output it cannot finish: a template that
-// fails partway through must yield a 500 and a body carrying none of
-// the content emitted before the failure. Against a renderer that
-// executes straight into the ResponseWriter this fails on both
-// counts, returning 200 with the prefix already flushed.
-func TestRenderTemplateMidRenderErrorSendsNoPartialBody(t *testing.T) {
-	t.Parallel()
-
-	var h *handlers.Handlers
-
-	app := newTestApp(t, &h)
-	app.RequireStart()
-
-	t.Cleanup(app.RequireStop)
-
-	h.AddTemplateForTest("failing.html", template.Must(
-		template.New("failing").Parse(
-			`{{.Data.Prefix}}{{.Data.Boom}}TAIL`,
-		),
-	))
-
-	req := httptest.NewRequestWithContext(
-		context.Background(), http.MethodGet, "/", nil)
-	w := httptest.NewRecorder()
-
-	h.RenderTemplateForTest(
-		w, req, "failing.html", midRenderFailure{},
-	)
-
-	assert.Equal(
-		t, http.StatusInternalServerError, w.Code,
-		"a failed render must report a 500",
-	)
-	assert.Equal(
-		t, "Internal server error\n", w.Body.String(),
-		"the response must carry no part of the aborted page",
-	)
-}
-
 func TestBuildDatabaseTargetConfig_Valid(t *testing.T) {
 	t.Parallel()
 

internal/handlers/source_detail_test.go

@@ -131,6 +131,47 @@ func TestHandleSourceDetail_MasksSlackWebhookURL(t *testing.T) {
 	assert.Contains(t, body, "https://hooks.slack.com/...")
 }
 
+// TestHandleSourceDetail_MasksHTTPDestinationURL is the
+// regression test for the same leak reached through the http
+// target: its destination is routinely an incoming-webhook
+// endpoint whose path segments are the credential, so the
+// rendered page must not contain them.
+func TestHandleSourceDetail_MasksHTTPDestinationURL(
+	t *testing.T,
+) {
+	t.Parallel()
+
+	var (
+		h    *handlers.Handlers
+		sess *session.Session
+		db   *database.Database
+	)
+
+	app := newTestApp(t, &h, &sess, &db)
+	app.RequireStart()
+
+	t.Cleanup(app.RequireStop)
+
+	wh := seedWebhook(t, db)
+	seedConfiguredTarget(
+		t, db, wh.ID,
+		database.TargetTypeHTTP,
+		`{"url":"`+slackWebhookURL+`"}`,
+	)
+
+	body := renderSourceDetailPage(t, h, sess, wh.ID)
+
+	assert.NotContains(t, body, slackSecretPath)
+	assert.NotContains(t, body, "T00000000")
+	assert.NotContains(t, body, "B00000000")
+	assert.NotContains(
+		t, body, "XXXXXXXXXXXXXXXXXXXXXXXX",
+	)
+
+	assert.Contains(t, body, "Destination URL")
+	assert.Contains(t, body, "https://hooks.slack.com/...")
+}
+
 // TestHandleSourceDetail_RendersNamedTargetFields proves the
 // other target types render labelled fields rather than the
 // stored blob.
@@ -172,7 +213,7 @@ func TestHandleSourceDetail_RendersNamedTargetFields(
 	body := renderSourceDetailPage(t, h, sess, wh.ID)
 
 	assert.Contains(t, body, "Destination URL")
-	assert.Contains(t, body, "https://example.com/hook")
+	assert.Contains(t, body, "https://example.com/...")
 	assert.Contains(t, body, "Timeout")
 	assert.Contains(t, body, "1 configured")
 	assert.NotContains(t, body, "sekrit")