Compare commits

..
2 Commits
Author SHA1 Message Date
clawbot 63c4efb639 resolver: a nameserver is asked at each of its addresses in turn (closes #285)
check / check (push) Canceled after 0s
A nameserver's queries went only to the first IPv4 address its name
resolves to, so one unreachable address made it fail whenever that
address came first in the answer. Each record type is now asked at the
addresses one at a time until one gives a usable reply, and the next
type is asked first at the address that gave it. A type that fails is
logged with the reason at each address, and a nameserver is reported as
timed out only when every query timed out; one given no address has
failed. The README's DNS Resolution Strategy says which addresses are
asked.

Model: opus-5-5
2026-10-15 13:18:49 +00:00
clawbot 5e2c456e85 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
2026-10-15 14:48:17 +02:00
6 changed files with 270 additions and 10 deletions

No files matched your search

+2
View File
@@ -21,6 +21,8 @@ milestone: https://git.eeqj.de/sneak/dnswatcher/milestone/7
- 2026-10-15: a nameserver is asked at each of its IPv4 addresses in turn, so it
has failed only when none of them gives a usable reply (closes #285).
- 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()
+51
View File
@@ -621,6 +621,57 @@ func TestQueryNameserver_Refused(t *testing.T) {
assert.Equal(t, "server returned REFUSED", resp.Error)
}
// TestQueryNameserver_EveryAddressRefuses asks ada.ns.cloudflare.com,
// whose name has several IPv4 addresses, about google.com, a zone it
// does not serve, which it refuses. A record type is asked at each of
// the nameserver's addresses before it fails, so the warning about it
// names every one.
func TestQueryNameserver_EveryAddressRefuses(t *testing.T) {
t.Parallel()
const ns = "ada.ns.cloudflare.com."
var logs bytes.Buffer
r := resolver.NewFromLogger(slog.New(slog.NewTextHandler(&logs, nil)))
nsIPs := liveResolveNSIPs(t, r, []string{ns}, 2)
var resp *resolver.NameserverResponse
livednstest.Retry(
t,
"QueryNameserver("+ns+", google.com)",
func(ctx context.Context) error {
logs.Reset()
var err error
resp, err = r.QueryNameserver(ctx, ns, "google.com")
if err != nil {
return err
}
// A timeout or a network error at every address is no
// reply at all.
if resp.Status == resolver.StatusTimeout ||
strings.HasPrefix(resp.Error, "network error") {
return fmt.Errorf(
"%w: %s: %s", livednstest.ErrNoAnswer, ns, resp.Error,
)
}
return nil
},
)
assert.Equal(t, resolver.StatusError, resp.Status)
assert.Equal(t, "server returned REFUSED", resp.Error)
for _, ip := range nsIPs {
assert.Contains(t, logs.String(), "@"+ip+": ")
}
}
// TestQueryServers_RecursiveResolverRefused passes a public recursive
// resolver to QueryServers as the server of google.com. These resolvers
// refuse a query that does not ask for recursion and answer one that