Mask target config on the source detail page (closes #113)

The source detail page rendered each target's stored config
blob verbatim. For a slack target that blob contains the
incoming webhook URL, which is a bearer credential: anyone
holding it can post to the channel indefinitely, and it
cannot be scoped or revoked per holder. Rendering it put the
credential into browser history, screenshots and any support
screen share.

Targets are now projected to a display-safe TargetView that
has no raw config field at all, so no template can render the
blob. Each type contributes named fields instead: slack shows
only a masked webhook URL, http shows its destination,
timeout, header count and retry settings, and database shows
its archive expiry. Header values are not shown because they
routinely carry authorization tokens.

Masking is a method on the config type,
SlackTargetConfig.MaskedWebhookURL, so it is unit-testable
and cannot be bypassed from a template. It reduces the URL to
scheme and host, eliding the path, query and any userinfo:
the field accepts an arbitrary URL, so no path segment can be
assumed non-secret. Any config that is empty, of an unknown
type, or fails to parse renders a neutral placeholder — there
is no fallback to the stored string on any path.

The stored config format and the delivery path are unchanged.

internal/delivery/target_config_view.go

@@ -0,0 +1,224 @@
+package delivery
+
+import (
+	"encoding/json"
+	"fmt"
+	"net/url"
+	"strconv"
+
+	"sneak.berlin/go/webhooker/internal/database"
+)
+
+// configUnavailable is what a target's configuration renders
+// as when it is absent, of an unknown type, or does not
+// parse. The stored blob is never shown as a fallback: it can
+// hold a credential (a Slack incoming webhook URL is a bearer
+// token) and a UI that prints it leaks that credential into
+// browser history, screenshots and screen shares.
+const configUnavailable = "(unavailable)"
+
+// urlPathElision stands in for a URL's elided path.
+const urlPathElision = "/..."
+
+// ConfigField is one labelled, display-safe value derived
+// from a target's stored configuration.
+type ConfigField struct {
+	Label string
+	Value string
+}
+
+// TargetView is the display-safe projection of a target for
+// the UI. It deliberately has no raw configuration field, so
+// no template — present or future — can render the stored
+// blob.
+type TargetView struct {
+	ID     string
+	Name   string
+	Type   database.TargetType
+	Active bool
+	Config []ConfigField
+}
+
+// NewTargetViews projects targets for rendering, replacing
+// each stored configuration blob with named, display-safe
+// fields.
+func NewTargetViews(
+	targets []database.Target,
+) []TargetView {
+	views := make([]TargetView, 0, len(targets))
+
+	for i := range targets {
+		t := &targets[i]
+
+		views = append(views, TargetView{
+			ID:     t.ID,
+			Name:   t.Name,
+			Type:   t.Type,
+			Active: t.Active,
+			Config: targetConfigFields(t),
+		})
+	}
+
+	return views
+}
+
+// targetConfigFields returns the display-safe fields for a
+// target's configuration. Anything it cannot parse becomes
+// the neutral placeholder.
+func targetConfigFields(
+	t *database.Target,
+) []ConfigField {
+	switch t.Type {
+	case database.TargetTypeSlack:
+		return slackConfigFields(t.Config)
+	case database.TargetTypeHTTP:
+		return httpConfigFields(t)
+	case database.TargetTypeDatabase:
+		return databaseConfigFields(t.Config)
+	case database.TargetTypeLog:
+		// The log target takes no configuration.
+		return nil
+	default:
+		return unavailableConfigFields()
+	}
+}
+
+// unavailableConfigFields is the neutral placeholder shown
+// for a configuration that could not be presented.
+func unavailableConfigFields() []ConfigField {
+	return []ConfigField{{
+		Label: "Configuration",
+		Value: configUnavailable,
+	}}
+}
+
+// slackConfigFields describes a Slack target. Only the masked
+// webhook URL is shown; the full URL is the credential.
+func slackConfigFields(configJSON string) []ConfigField {
+	cfg, err := parseSlackConfig(configJSON)
+	if err != nil {
+		return unavailableConfigFields()
+	}
+
+	return []ConfigField{{
+		Label: "Webhook URL",
+		Value: cfg.MaskedWebhookURL(),
+	}}
+}
+
+// httpConfigFields describes an HTTP target: its destination
+// and its retry settings. Header values are not shown — they
+// routinely carry authorization tokens — only how many are
+// configured.
+func httpConfigFields(t *database.Target) []ConfigField {
+	cfg, err := parseHTTPConfig(t.Config)
+	if err != nil {
+		return unavailableConfigFields()
+	}
+
+	fields := []ConfigField{{
+		Label: "Destination URL",
+		Value: cfg.URL,
+	}}
+
+	if cfg.Timeout > 0 {
+		fields = append(fields, ConfigField{
+			Label: "Timeout",
+			Value: strconv.Itoa(cfg.Timeout) + "s",
+		})
+	}
+
+	if len(cfg.Headers) > 0 {
+		fields = append(fields, ConfigField{
+			Label: "Headers",
+			Value: fmt.Sprintf(
+				"%d configured", len(cfg.Headers),
+			),
+		})
+	}
+
+	return append(fields, retryFields(t)...)
+}
+
+// retryFields describes a target's retry settings, which live
+// on the target row rather than in its configuration blob.
+func retryFields(t *database.Target) []ConfigField {
+	retries := strconv.Itoa(t.MaxRetries)
+	if t.MaxRetries == 0 {
+		retries += " (fire-and-forget)"
+	}
+
+	fields := []ConfigField{{
+		Label: "Max Retries",
+		Value: retries,
+	}}
+
+	if t.MaxQueueSize > 0 {
+		fields = append(fields, ConfigField{
+			Label: "Max Queue Size",
+			Value: strconv.Itoa(t.MaxQueueSize),
+		})
+	}
+
+	return fields
+}
+
+// databaseConfigFields describes an archive target. Its
+// configuration is optional, and an absent or empty expiry
+// means the archive is kept forever. An expiry that is set
+// but not a valid duration is reported as unavailable rather
+// than echoed back.
+func databaseConfigFields(configJSON string) []ConfigField {
+	expiry := archiveExpiryNever
+
+	if configJSON != "" {
+		var cfg databaseTargetConfig
+
+		err := json.Unmarshal([]byte(configJSON), &cfg)
+		if err != nil {
+			return unavailableConfigFields()
+		}
+
+		if cfg.Expiry != "" {
+			if ValidateArchiveExpiry(cfg.Expiry) != nil {
+				return unavailableConfigFields()
+			}
+
+			expiry = cfg.Expiry
+		}
+	}
+
+	return []ConfigField{{
+		Label: "Archive Expiry",
+		Value: expiry,
+	}}
+}
+
+// MaskedWebhookURL returns the Slack webhook URL reduced to
+// its scheme and host, with the path, query and any userinfo
+// elided. The path segments are the credential, so none of
+// them is shown: the field accepts an arbitrary URL, so no
+// segment can be assumed non-secret. A URL that does not
+// parse into a scheme and host yields the neutral
+// placeholder, never the raw string.
+func (c *SlackTargetConfig) MaskedWebhookURL() string {
+	return maskURL(c.WebhookURL)
+}
+
+// maskURL renders a URL as scheme plus host with everything
+// that can carry a secret removed.
+func maskURL(raw string) string {
+	parsed, err := url.Parse(raw)
+	if err != nil || parsed.Scheme == "" ||
+		parsed.Host == "" {
+		return configUnavailable
+	}
+
+	masked := parsed.Scheme + "://" + parsed.Host
+
+	if parsed.Path != "" && parsed.Path != "/" {
+		masked += urlPathElision
+	}
+
+	return masked
+}

internal/delivery/target_config_view_test.go

@@ -0,0 +1,299 @@
+package delivery_test
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"sneak.berlin/go/webhooker/internal/database"
+	"sneak.berlin/go/webhooker/internal/delivery"
+)
+
+const (
+	// slackSecretPath is the credential-bearing part of a
+	// Slack incoming webhook URL: everything after the host.
+	slackSecretPath = "/services/T00000000/B00000000/" +
+		"XXXXXXXXXXXXXXXXXXXXXXXX"
+	slackWebhookURL = "https://hooks.slack.com" +
+		slackSecretPath
+
+	viewExampleOrigin = "https://example.com"
+	viewExampleHook   = viewExampleOrigin + "/hook"
+	viewUnavailable   = "(unavailable)"
+	viewExpiryNever   = "never"
+)
+
+func TestMaskedWebhookURL(t *testing.T) {
+	t.Parallel()
+
+	tests := map[string]struct {
+		url  string
+		want string
+	}{
+		"slack webhook": {
+			url:  slackWebhookURL,
+			want: "https://hooks.slack.com/...",
+		},
+		"query string dropped": {
+			url:  viewExampleOrigin + "/a?token=secret",
+			want: viewExampleOrigin + "/...",
+		},
+		// Fabricated userinfo in a test URL, not a real
+		// credential.
+		//nolint:gosec // G101
+		"userinfo dropped": {
+			url:  "https://user:pw@example.com/a/b",
+			want: viewExampleOrigin + "/...",
+		},
+		"no path": {
+			url:  viewExampleOrigin,
+			want: viewExampleOrigin,
+		},
+		"root path": {
+			url:  viewExampleOrigin + "/",
+			want: viewExampleOrigin,
+		},
+		"not a url": {
+			url:  "definitely not a url",
+			want: viewUnavailable,
+		},
+		"empty": {
+			url:  "",
+			want: viewUnavailable,
+		},
+	}
+
+	for name, tc := range tests {
+		t.Run(name, func(t *testing.T) {
+			t.Parallel()
+
+			cfg := &delivery.SlackTargetConfig{
+				WebhookURL: tc.url,
+			}
+
+			assert.Equal(
+				t, tc.want, cfg.MaskedWebhookURL(),
+			)
+		})
+	}
+}
+
+// TestMaskedWebhookURL_NeverLeaksPath is the direct
+// expression of the rule: whatever the input, the masked
+// value never contains a path segment of it.
+func TestMaskedWebhookURL_NeverLeaksPath(t *testing.T) {
+	t.Parallel()
+
+	cfg := &delivery.SlackTargetConfig{
+		WebhookURL: slackWebhookURL,
+	}
+
+	masked := cfg.MaskedWebhookURL()
+
+	assert.NotContains(t, masked, "T00000000")
+	assert.NotContains(t, masked, "B00000000")
+	assert.NotContains(
+		t, masked, "XXXXXXXXXXXXXXXXXXXXXXXX",
+	)
+	assert.NotContains(t, masked, slackSecretPath)
+}
+
+// fieldMap turns a view's config fields into a lookup so
+// assertions read by label.
+func fieldMap(fields []delivery.ConfigField) map[string]string {
+	out := make(map[string]string, len(fields))
+	for _, f := range fields {
+		out[f.Label] = f.Value
+	}
+
+	return out
+}
+
+// viewFor projects a single target and returns its view.
+func viewFor(
+	t *testing.T,
+	target database.Target,
+) delivery.TargetView {
+	t.Helper()
+
+	views := delivery.NewTargetViews(
+		[]database.Target{target},
+	)
+	require.Len(t, views, 1)
+
+	return views[0]
+}
+
+func TestNewTargetViews_Slack(t *testing.T) {
+	t.Parallel()
+
+	view := viewFor(t, database.Target{
+		Name:   "slack-target",
+		Type:   database.TargetTypeSlack,
+		Active: true,
+		Config: `{"webhookUrl":"` +
+			slackWebhookURL + `"}`,
+	})
+
+	assert.Equal(t, "slack-target", view.Name)
+	assert.Equal(
+		t,
+		map[string]string{
+			"Webhook URL": "https://hooks.slack.com/...",
+		},
+		fieldMap(view.Config),
+	)
+}
+
+func TestNewTargetViews_HTTP(t *testing.T) {
+	t.Parallel()
+
+	view := viewFor(t, database.Target{
+		Type: database.TargetTypeHTTP,
+		Config: `{"url":"` + viewExampleHook + `",` +
+			`"timeout":30,` +
+			`"headers":{"Authorization":"Bearer sekrit"}}`,
+		MaxRetries:   5,
+		MaxQueueSize: 100,
+	})
+
+	fields := fieldMap(view.Config)
+
+	assert.Equal(
+		t,
+		map[string]string{
+			"Destination URL": viewExampleHook,
+			"Timeout":         "30s",
+			"Headers":         "1 configured",
+			"Max Retries":     "5",
+			"Max Queue Size":  "100",
+		},
+		fields,
+	)
+
+	// Header values can be credentials and are never shown.
+	for _, v := range fields {
+		assert.NotContains(t, v, "sekrit")
+	}
+}
+
+func TestNewTargetViews_HTTPFireAndForget(t *testing.T) {
+	t.Parallel()
+
+	view := viewFor(t, database.Target{
+		Type:   database.TargetTypeHTTP,
+		Config: `{"url":"` + viewExampleHook + `"}`,
+	})
+
+	assert.Equal(
+		t,
+		map[string]string{
+			"Destination URL": viewExampleHook,
+			"Max Retries":     "0 (fire-and-forget)",
+		},
+		fieldMap(view.Config),
+	)
+}
+
+func TestNewTargetViews_Database(t *testing.T) {
+	t.Parallel()
+
+	tests := map[string]struct {
+		config string
+		want   string
+	}{
+		"empty config": {config: "", want: viewExpiryNever},
+		"empty expiry": {config: `{}`, want: viewExpiryNever},
+		"explicit": {
+			config: `{"expiry":"720h"}`,
+			want:   "720h",
+		},
+		"never literal": {
+			config: `{"expiry":"` + viewExpiryNever + `"}`,
+			want:   viewExpiryNever,
+		},
+	}
+
+	for name, tc := range tests {
+		t.Run(name, func(t *testing.T) {
+			t.Parallel()
+
+			view := viewFor(t, database.Target{
+				Type:   database.TargetTypeDatabase,
+				Config: tc.config,
+			})
+
+			assert.Equal(
+				t,
+				map[string]string{"Archive Expiry": tc.want},
+				fieldMap(view.Config),
+			)
+		})
+	}
+}
+
+func TestNewTargetViews_Log(t *testing.T) {
+	t.Parallel()
+
+	view := viewFor(t, database.Target{
+		Type:   database.TargetTypeLog,
+		Config: "",
+	})
+
+	assert.Empty(t, view.Config)
+}
+
+// TestNewTargetViews_Unpresentable proves that no config the
+// view cannot present falls back to the stored blob.
+func TestNewTargetViews_Unpresentable(t *testing.T) {
+	t.Parallel()
+
+	const blob = `{"webhookUrl":"https://hooks.slack.com` +
+		slackSecretPath + `"`
+
+	tests := map[string]database.Target{
+		"unknown target type": {
+			Type:   database.TargetType("carrier-pigeon"),
+			Config: blob,
+		},
+		"unparseable json": {
+			Type:   database.TargetTypeSlack,
+			Config: blob,
+		},
+		"empty slack config": {
+			Type: database.TargetTypeSlack,
+		},
+		"slack config without url": {
+			Type:   database.TargetTypeSlack,
+			Config: `{}`,
+		},
+		"unparseable http json": {
+			Type:   database.TargetTypeHTTP,
+			Config: `{"url":`,
+		},
+		"unparseable archive json": {
+			Type:   database.TargetTypeDatabase,
+			Config: `{"expiry":`,
+		},
+		"invalid archive expiry": {
+			Type:   database.TargetTypeDatabase,
+			Config: `{"expiry":"a fortnight"}`,
+		},
+	}
+
+	for name, target := range tests {
+		t.Run(name, func(t *testing.T) {
+			t.Parallel()
+
+			view := viewFor(t, target)
+
+			assert.Equal(
+				t,
+				map[string]string{
+					"Configuration": viewUnavailable,
+				},
+				fieldMap(view.Config),
+			)
+		})
+	}
+}

internal/handlers/source_detail_test.go

@@ -0,0 +1,185 @@
+package handlers_test
+
+import (
+	"context"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"github.com/go-chi/chi"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"gorm.io/gorm/clause"
+	"sneak.berlin/go/webhooker/internal/database"
+	"sneak.berlin/go/webhooker/internal/handlers"
+	"sneak.berlin/go/webhooker/internal/session"
+)
+
+// The secret path segments of a Slack incoming webhook URL.
+// Holding them is enough to post to the channel forever, so
+// they must never reach the rendered page.
+const (
+	slackSecretPath = "/services/T00000000/B00000000/" +
+		"XXXXXXXXXXXXXXXXXXXXXXXX"
+	slackWebhookURL = "https://hooks.slack.com" +
+		slackSecretPath
+)
+
+// seedConfiguredTarget inserts a target with a stored config
+// blob.
+func seedConfiguredTarget(
+	t *testing.T,
+	db *database.Database,
+	webhookID string,
+	targetType database.TargetType,
+	config string,
+) {
+	t.Helper()
+
+	tgt := &database.Target{
+		WebhookID: webhookID,
+		Name:      "t-" + string(targetType),
+		Type:      targetType,
+		Active:    true,
+		Config:    config,
+	}
+
+	require.NoError(
+		t,
+		db.DB().Omit(clause.Associations).Create(tgt).Error,
+	)
+}
+
+// renderSourceDetailPage runs the real source detail handler
+// for a webhook and returns the rendered HTML.
+func renderSourceDetailPage(
+	t *testing.T,
+	h *handlers.Handlers,
+	sess *session.Session,
+	webhookID string,
+) string {
+	t.Helper()
+
+	req := httptest.NewRequestWithContext(
+		context.Background(),
+		http.MethodGet,
+		"/source/"+webhookID,
+		nil,
+	)
+
+	for _, c := range authenticatedCookies(
+		t, sess, deleteTestUserID, deleteTestUsername,
+	) {
+		req.AddCookie(c)
+	}
+
+	rctx := chi.NewRouteContext()
+	rctx.URLParams.Add(paramSourceID, webhookID)
+
+	req = req.WithContext(
+		context.WithValue(
+			req.Context(), chi.RouteCtxKey, rctx,
+		),
+	)
+
+	w := httptest.NewRecorder()
+	h.HandleSourceDetail().ServeHTTP(w, req)
+
+	require.Equal(t, http.StatusOK, w.Code)
+
+	return w.Body.String()
+}
+
+// TestHandleSourceDetail_MasksSlackWebhookURL is the
+// load-bearing regression test for the credential leak: the
+// rendered page must show the Slack target without any of the
+// secret path segments of its webhook URL.
+func TestHandleSourceDetail_MasksSlackWebhookURL(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.TargetTypeSlack,
+		`{"webhookUrl":"`+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.NotContains(t, body, "webhookUrl")
+
+	assert.Contains(t, body, "Webhook URL")
+	assert.Contains(t, body, "https://hooks.slack.com/...")
+}
+
+// TestHandleSourceDetail_RendersNamedTargetFields proves the
+// other target types render labelled fields rather than the
+// stored blob.
+func TestHandleSourceDetail_RendersNamedTargetFields(
+	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":"https://example.com/hook","timeout":30,`+
+			`"headers":{"Authorization":"Bearer sekrit"}}`,
+	)
+	seedConfiguredTarget(
+		t, db, wh.ID,
+		database.TargetTypeDatabase,
+		`{"expiry":"720h"}`,
+	)
+	seedConfiguredTarget(
+		t, db, wh.ID,
+		database.TargetType("carrier-pigeon"),
+		`{"beak":"sharp"}`,
+	)
+
+	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, "Timeout")
+	assert.Contains(t, body, "1 configured")
+	assert.NotContains(t, body, "sekrit")
+
+	assert.Contains(t, body, "Archive Expiry")
+	assert.Contains(t, body, "720h")
+
+	// An unknown type gets the neutral placeholder, never the
+	// stored blob.
+	assert.Contains(t, body, "(unavailable)")
+	assert.NotContains(t, body, "beak")
+}

internal/handlers/source_management.go

@@ -318,9 +318,12 @@ func (h *Handlers) renderSourceDetail(
 	data := map[string]any{
 		tmplKeyWebhook: webhook,
 		"Entrypoints":  entrypoints,
-		"Targets":      targets,
+		// Targets are projected to a display-safe view: the
-		"Events":       events,
+		// stored config blob holds credentials and must never
-		"BaseURL":      scheme + "://" + host,
+		// reach a template.
+		"Targets": delivery.NewTargetViews(targets),
+		"Events":  events,
+		"BaseURL": scheme + "://" + host,
 	}
 
 	h.renderTemplate(w, r, "source_detail.html", data)

templates/source_detail.html

@@ -145,8 +145,11 @@
                             </form>
                         </div>
                     </div>
-                    {{if .Config}}
+                    {{range .Config}}
-                    <code class="text-xs text-gray-500 break-all block mt-1">{{.Config}}</code>
+                    <div class="text-xs text-gray-500 break-all mt-1">
+                        <span class="font-medium text-gray-700">{{.Label}}:</span>
+                        <span>{{.Value}}</span>
+                    </div>
                     {{end}}
                 </div>
                 {{else}}