notify: the log names a notification endpoint, not its webhook URL (closes #288)
check / check (push) Canceled after 0s
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 is contained in:
@@ -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) {
|
||||
|
||||
Reference in New Issue
Block a user