notify: the log names a notification endpoint, not its webhook URL (closes #288)
check / check (push) Canceled after 0s

With debug logging on, each delivery logged the full Slack, Mattermost
or ntfy URL, whose path or query lets anyone who has it post to the
channel or topic. The two debug lines now give the endpoint's name and
the title; sendSlack is told which of Slack or Mattermost it sends to.
A webhook URL that does not parse gave a startup error quoting the
whole URL; it now gives only the reason. Errors from a failed request
come from the transport, not from an HTTP client, and carry no part of
the path or query; a test now checks that too.

Model: opus-5-5
This commit was merged in pull request #294.
This commit is contained in:
2026-10-15 14:48:17 +02:00
parent b79726acc6
commit 5e2c456e85
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 # 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 - 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). 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 - 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" "encoding/json"
"errors" "errors"
"io" "io"
"log/slog"
"maps" "maps"
"net/http" "net/http"
"net/http/httptest" "net/http/httptest"
"net/url" "net/url"
"slices"
"strings" "strings"
"sync" "sync"
"testing" "testing"
@@ -38,6 +40,13 @@ const (
prioUrgent = "urgent" 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. // testHost is the hostname used in request construction tests.
const testHost = "example.com" 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 ────────────────────────── // ── SlackPayload JSON marshaling ──────────────────────────
func TestSlackPayloadJSON(t *testing.T) { 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) 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( func (svc *Service) SendSlack(
ctx context.Context, ctx context.Context,
webhookURL *url.URL, webhookURL *url.URL,
@@ -89,7 +90,7 @@ func (svc *Service) SendSlack(
title, message, priority string, title, message, priority string,
) error { ) error {
return svc.sendSlack( 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) { func ValidateWebhookURL(raw string) (*url.URL, error) {
u, err := url.ParseRequestURI(raw) u, err := url.ParseRequestURI(raw)
if err != nil { 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) 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 { svc.dispatch(ctx, "slack", func(c context.Context) error {
return svc.sendSlack( return svc.sendSlack(
c, svc.slackWebhookURL, ErrSlackFailed, c, svc.slackWebhookURL, "slack", ErrSlackFailed,
title, message, priority, title, message, priority,
) )
}) })
@@ -295,8 +304,8 @@ func (svc *Service) dispatchMattermost(
ctx, "mattermost", ctx, "mattermost",
func(c context.Context) error { func(c context.Context) error {
return svc.sendSlack( return svc.sendSlack(
c, svc.mattermostWebhookURL, ErrMattermostFailed, c, svc.mattermostWebhookURL, "mattermost",
title, message, priority, ErrMattermostFailed, title, message, priority,
) )
}, },
) )
@@ -307,9 +316,11 @@ func (svc *Service) sendNtfy(
topicURL *url.URL, topicURL *url.URL,
title, message, priority string, title, message, priority string,
) error { ) 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( svc.log.Debug(
"sending ntfy notification", "sending ntfy notification",
"topic", topicURL.String(), "endpoint", "ntfy",
"title", title, "title", title,
) )
@@ -372,12 +383,15 @@ type SlackAttachment struct {
} }
// sendSlack posts to a Slack or Mattermost incoming webhook, which // sendSlack posts to a Slack or Mattermost incoming webhook, which
// take the same payload. An HTTP error status is returned wrapped in // take the same payload. endpoint, "slack" or "mattermost", names
// failed, ErrSlackFailed or ErrMattermostFailed, so the error names // the webhook in the log in place of its URL, which lets anyone who
// the endpoint. // 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( func (svc *Service) sendSlack(
ctx context.Context, ctx context.Context,
webhookURL *url.URL, webhookURL *url.URL,
endpoint string,
failed error, failed error,
title, message, priority string, title, message, priority string,
) error { ) error {
@@ -388,7 +402,7 @@ func (svc *Service) sendSlack(
svc.log.Debug( svc.log.Debug(
"sending webhook notification", "sending webhook notification",
"url", webhookURL.String(), "endpoint", endpoint,
"title", title, "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) { func TestIsAllowedScheme(t *testing.T) {
t.Parallel() t.Parallel()