notify: the log names a notification endpoint, not its webhook URL #294

Merged
clawbot merged 1 commits from issue-288-webhook-url-in-log into next 2026-10-15 14:48:18 +02:00
5 changed files with 219 additions and 10 deletions
+2
View File
@@ -19,6 +19,8 @@ milestone: https://git.eeqj.de/sneak/dnswatcher/milestone/7
# Completed Steps
- 2026-10-15: the log names a notification endpoint (`slack`, `mattermost`,
`ntfy`) and never shows its URL's path or query (closes #288).
- 2026-10-13: a hostname or a domain whose zone's nameservers all stop answering
gets an NS Failure for each, then an NS Recovery (closes #284).
- 2026-10-13: an IPv6 address is written in brackets before its port, as in
+175
View File
@@ -6,10 +6,12 @@ import (
"encoding/json"
"errors"
"io"
"log/slog"
"maps"
"net/http"
"net/http/httptest"
"net/url"
"slices"
"strings"
"sync"
"testing"
@@ -38,6 +40,13 @@ const (
prioUrgent = "urgent"
)
// Endpoint names, as the log gives them, used across multiple tests.
const (
endpointNtfy = "ntfy"
endpointSlack = "slack"
endpointMattermost = "mattermost"
)
// testHost is the hostname used in request construction tests.
const testHost = "example.com"
@@ -1094,6 +1103,172 @@ func TestSendNotificationErrorNamesEndpoint(t *testing.T) {
}
}
// ── webhook URLs kept out of the log ──────────────────────
// webhookPathAndQuery follows the server's address in every
// endpoint's URL in the tests below. A webhook URL's path or query
// lets anyone post to the channel or topic, so neither word marked
// secret in it may reach the log.
const webhookPathAndQuery = "/hooks/secret-in-path?token=secret-in-query"
// assertNoWebhookSecret fails the test if text holds either word
// marked secret in webhookPathAndQuery.
func assertNoWebhookSecret(t *testing.T, text string) {
t.Helper()
for _, secret := range []string{"secret-in-path", "secret-in-query"} {
if strings.Contains(text, secret) {
t.Errorf("%q from the webhook URL is in:\n%s", secret, text)
}
}
}
// logRecord is the part of a JSON log line the tests below read.
type logRecord struct {
Level string `json:"level"`
Msg string `json:"msg"`
Endpoint string `json:"endpoint"`
Title string `json:"title"`
Error string `json:"error"`
}
// parseLog decodes each line of a JSON log.
func parseLog(t *testing.T, logs string) []logRecord {
t.Helper()
var records []logRecord
for line := range strings.Lines(logs) {
var record logRecord
err := json.Unmarshal([]byte(line), &record)
if err != nil {
t.Fatalf("decoding log line %q: %v", line, err)
}
records = append(records, record)
}
return records
}
// sendToEachEndpoint sends one notification with the given title to
// ntfy, Slack and Mattermost, each at baseURL followed by
// webhookPathAndQuery, and retries a failed delivery once. When
// every delivery has finished it returns the JSON log written,
// debug lines included.
func sendToEachEndpoint(t *testing.T, baseURL, title string) string {
t.Helper()
target, err := url.Parse(baseURL + webhookPathAndQuery)
if err != nil {
t.Fatalf("parsing webhook URL: %v", err)
}
svc, logs := newLoggingService(http.DefaultTransport)
svc.SetNtfyURL(target)
svc.SetSlackWebhookURL(target)
svc.SetMattermostWebhookURL(target)
svc.SetSleepFunc(instantSleep)
svc.SetRetryConfig(notify.RetryConfig{
MaxRetries: 1,
BaseDelay: time.Millisecond,
MaxDelay: time.Millisecond,
})
svc.SendNotification(context.Background(), title, "m", prioInfo)
waitForCondition(t, func() bool {
return svc.OutstandingDeliveries() == 0
})
return logs.String()
}
// TestSendNotificationDebugLogOmitsWebhookURL checks that, with debug
// logging on, a delivery to each endpoint logs the endpoint's name and
// the title, and no part of the URL's path or query.
func TestSendNotificationDebugLogOmitsWebhookURL(t *testing.T) {
t.Parallel()
const title = "Port Change"
srv := httptest.NewServer(
http.HandlerFunc(
func(w http.ResponseWriter, _ *http.Request) {
w.WriteHeader(http.StatusOK)
}),
)
defer srv.Close()
logs := sendToEachEndpoint(t, srv.URL, title)
assertNoWebhookSecret(t, logs)
got := map[string]string{}
for _, record := range parseLog(t, logs) {
if record.Level == slog.LevelDebug.String() {
got[record.Endpoint] = record.Title
}
}
want := map[string]string{
endpointNtfy: title,
endpointSlack: title,
endpointMattermost: title,
}
if !maps.Equal(got, want) {
t.Errorf("debug lines' endpoint and title = %v, want %v", got, want)
}
}
// TestSendNotificationErrorLogOmitsWebhookURL checks that a delivery
// to each endpoint that cannot connect logs the transport's error,
// which names the refused connection, on its retry line and on its
// final failure line, and that no line holds any part of the URL's
// path or query.
func TestSendNotificationErrorLogOmitsWebhookURL(t *testing.T) {
t.Parallel()
// Closed before anything is sent, so every attempt fails to
// connect.
srv := httptest.NewServer(http.NotFoundHandler())
srv.Close()
logs := sendToEachEndpoint(t, srv.URL, "t")
assertNoWebhookSecret(t, logs)
// For each endpoint, the messages of its lines whose error names
// the refused connection, in the order they were logged.
got := map[string][]string{}
for _, record := range parseLog(t, logs) {
if strings.Contains(record.Error, "connection refused") {
got[record.Endpoint] = append(
got[record.Endpoint], record.Msg,
)
}
}
lines := []string{
"notification delivery failed, retrying",
"failed to send notification after retries",
}
want := map[string][]string{
endpointNtfy: lines,
endpointSlack: lines,
endpointMattermost: lines,
}
if !maps.EqualFunc(got, want, slices.Equal) {
t.Errorf("lines whose error names the refused connection = %v, "+
"want %v", got, want)
}
}
// ── SlackPayload JSON marshaling ──────────────────────────
func TestSlackPayloadJSON(t *testing.T) {
+3 -2
View File
@@ -81,7 +81,8 @@ func (svc *Service) SendNtfy(
return svc.sendNtfy(ctx, topicURL, title, message, priority)
}
// SendSlack exports sendSlack for testing.
// SendSlack exports sendSlack for testing. It sends as the
// "slack" endpoint.
func (svc *Service) SendSlack(
ctx context.Context,
webhookURL *url.URL,
@@ -89,7 +90,7 @@ func (svc *Service) SendSlack(
title, message, priority string,
) error {
return svc.sendSlack(
ctx, webhookURL, failed, title, message, priority,
ctx, webhookURL, "slack", failed, title, message, priority,
)
}
+22 -8
View File
@@ -56,6 +56,15 @@ func IsAllowedScheme(scheme string) bool {
func ValidateWebhookURL(raw string) (*url.URL, error) {
u, err := url.ParseRequestURI(raw)
if err != nil {
// A *url.Error quotes the whole URL, and this error is
// logged at startup. A webhook URL's path or query lets
// anyone post to the channel or topic, so keep only the
// reason.
var urlErr *url.Error
if errors.As(err, &urlErr) {
err = urlErr.Err
}
return nil, fmt.Errorf("invalid URL: %w", err)
}
@@ -277,7 +286,7 @@ func (svc *Service) dispatchSlack(
svc.dispatch(ctx, "slack", func(c context.Context) error {
return svc.sendSlack(
c, svc.slackWebhookURL, ErrSlackFailed,
c, svc.slackWebhookURL, "slack", ErrSlackFailed,
title, message, priority,
)
})
@@ -295,8 +304,8 @@ func (svc *Service) dispatchMattermost(
ctx, "mattermost",
func(c context.Context) error {
return svc.sendSlack(
c, svc.mattermostWebhookURL, ErrMattermostFailed,
title, message, priority,
c, svc.mattermostWebhookURL, "mattermost",
ErrMattermostFailed, title, message, priority,
)
},
)
@@ -307,9 +316,11 @@ func (svc *Service) sendNtfy(
topicURL *url.URL,
title, message, priority string,
) error {
// The topic URL is left out of the log: it lets anyone who has
// it read and post to the topic.
svc.log.Debug(
"sending ntfy notification",
"topic", topicURL.String(),
"endpoint", "ntfy",
"title", title,
)
@@ -372,12 +383,15 @@ type SlackAttachment struct {
}
// sendSlack posts to a Slack or Mattermost incoming webhook, which
// take the same payload. An HTTP error status is returned wrapped in
// failed, ErrSlackFailed or ErrMattermostFailed, so the error names
// the endpoint.
// take the same payload. endpoint, "slack" or "mattermost", names
// the webhook in the log in place of its URL, which lets anyone who
// has it post to the channel. An HTTP error status is returned
// wrapped in failed, ErrSlackFailed or ErrMattermostFailed, so the
// error names the endpoint.
func (svc *Service) sendSlack(
ctx context.Context,
webhookURL *url.URL,
endpoint string,
failed error,
title, message, priority string,
) error {
@@ -388,7 +402,7 @@ func (svc *Service) sendSlack(
svc.log.Debug(
"sending webhook notification",
"url", webhookURL.String(),
"endpoint", endpoint,
"title", title,
)
+17
View File
@@ -79,6 +79,23 @@ func TestValidateWebhookURLInvalid(t *testing.T) {
}
}
// TestValidateWebhookURLErrorOmitsURL checks that the error for a
// webhook URL that does not parse, here because a carriage return
// was left at its end, leaves out the URL's path and query: the
// error is logged at startup.
func TestValidateWebhookURLErrorOmitsURL(t *testing.T) {
t.Parallel()
_, err := notify.ValidateWebhookURL(
"https://hooks.example.com" + webhookPathAndQuery + "\r",
)
if err == nil {
t.Fatal("expected an error for a URL ending in a carriage return")
}
assertNoWebhookSecret(t, err.Error())
}
func TestIsAllowedScheme(t *testing.T) {
t.Parallel()