diff --git a/internal/delivery/target_config_view.go b/internal/delivery/target_config_view.go index 8beb182..4174318 100644 --- a/internal/delivery/target_config_view.go +++ b/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 { diff --git a/internal/delivery/target_config_view_test.go b/internal/delivery/target_config_view_test.go index 0d5d910..c634d73 100644 --- a/internal/delivery/target_config_view_test.go +++ b/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() diff --git a/internal/handlers/source_detail_test.go b/internal/handlers/source_detail_test.go index dcc129c..fcaf698 100644 --- a/internal/handlers/source_detail_test.go +++ b/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")