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

The page rendered the stored target config verbatim, exposing the Slack
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.

Target config now reaches the template only as a TargetView carrying
labelled fields, so no code path can render the raw blob. maskURL keeps
scheme and host and elides the path, and drops query, fragment and
userinfo; every parse failure yields a neutral placeholder rather than
falling back to the stored string. HTTP header values are never rendered,
only a count.

Rendering change only: 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

@@ -414,9 +414,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}}