diff --git a/internal/delivery/target_config_view.go b/internal/delivery/target_config_view.go new file mode 100644 index 0000000..4a6b8b7 --- /dev/null +++ b/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 +} diff --git a/internal/delivery/target_config_view_test.go b/internal/delivery/target_config_view_test.go new file mode 100644 index 0000000..0d5d910 --- /dev/null +++ b/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), + ) + }) + } +} diff --git a/internal/handlers/source_detail_test.go b/internal/handlers/source_detail_test.go new file mode 100644 index 0000000..248362a --- /dev/null +++ b/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") +} diff --git a/internal/handlers/source_management.go b/internal/handlers/source_management.go index 4f295dc..42bad4c 100644 --- a/internal/handlers/source_management.go +++ b/internal/handlers/source_management.go @@ -318,9 +318,12 @@ func (h *Handlers) renderSourceDetail( data := map[string]any{ tmplKeyWebhook: webhook, "Entrypoints": entrypoints, - "Targets": targets, - "Events": events, - "BaseURL": scheme + "://" + host, + // Targets are projected to a display-safe view: the + // stored config blob holds credentials and must never + // reach a template. + "Targets": delivery.NewTargetViews(targets), + "Events": events, + "BaseURL": scheme + "://" + host, } h.renderTemplate(w, r, "source_detail.html", data) diff --git a/templates/source_detail.html b/templates/source_detail.html index 5b9944a..8f7909e 100644 --- a/templates/source_detail.html +++ b/templates/source_detail.html @@ -145,8 +145,11 @@ - {{if .Config}} - {{.Config}} + {{range .Config}} +
+ {{.Label}}: + {{.Value}} +
{{end}} {{else}}