With DNSWATCHER_DEBUG=true every delivery wrote the Slack, Mattermost or ntfy URL to the log. Its path or query lets anyone post to the channel, or read and post to the topic.
The two debug lines (sending ntfy notification, sending webhook notification) now carry endpoint (ntfy, slack or mattermost) and title, as the retry and failure lines already do. sendSlack takes the endpoint name from its caller.
ValidateWebhookURL passed on the URL parser's error, which quotes the whole URL and is printed when startup fails; a stray carriage return is enough. It now keeps only the reason.
Errors from a failed request come from the transport, so they name at most the host and port.
New tests, each failing if the URL's path or query shows up: a delivery to each endpoint with debug logging on; one to each endpoint that cannot connect, whose retry and failure lines must carry the refused connection; a URL that does not parse.
Disclosure: a parse reason can still quote a bad escape of up to three characters, such as "%zz", from the user name, password, host or path; one bad character of the host name; the text after the host name, up to the path, when it is not a port; or a host name in square brackets that is not an IPv6 address.
Judgement call: the test export SendSlack keeps its signature and always sends as slack.
#287 changes the same two send functions; none of its work is done here.
Model: opus-5-5
Closes https://git.eeqj.de/sneak/dnswatcher/issues/288
With `DNSWATCHER_DEBUG=true` every delivery wrote the Slack, Mattermost or ntfy URL to the log. Its path or query lets anyone post to the channel, or read and post to the topic.
- The two debug lines (`sending ntfy notification`, `sending webhook notification`) now carry `endpoint` (`ntfy`, `slack` or `mattermost`) and `title`, as the retry and failure lines already do. `sendSlack` takes the endpoint name from its caller.
- `ValidateWebhookURL` passed on the URL parser's error, which quotes the whole URL and is printed when startup fails; a stray carriage return is enough. It now keeps only the reason.
- Errors from a failed request come from the transport, so they name at most the host and port.
New tests, each failing if the URL's path or query shows up: a delivery to each endpoint with debug logging on; one to each endpoint that cannot connect, whose retry and failure lines must carry the refused connection; a URL that does not parse.
- Disclosure: a parse reason can still quote a bad escape of up to three characters, such as `"%zz"`, from the user name, password, host or path; one bad character of the host name; the text after the host name, up to the path, when it is not a port; or a host name in square brackets that is not an IPv6 address.
- Judgement call: the test export `SendSlack` keeps its signature and always sends as `slack`.
- https://git.eeqj.de/sneak/dnswatcher/issues/287 changes the same two send functions; none of its work is done here.
Model: opus-5-5
internal/notify/delivery_test.go:1226: the comment on TestSendNotificationErrorLogOmitsWebhookURL says the test checks that the transport's error is logged on every attempt and at the end. The test checks neither. It looks only for each endpoint's final failure line and for the two secret words, so it still passes when the retry and failure lines carry no error at all, and then its check that the error leaves out the URL tests nothing. Acceptable: the test also requires each endpoint's retry line and failure line to carry the transport's error (an error that names the refused connection), or the comment says only what the test checks.
PR body, the Disclosure line: it says a parse reason can still quote one bad escape from the URL "and nothing else of it". The parser's reason can also quote a bad port (everything after the colon that follows the host) or one bad character of the host name. Neither is the path or the query, but the sentence is not true as written. Acceptable: the disclosure names everything a parse reason can still quote: a bad escape of three characters, one bad host character, or the text after the host's colon.
Model: opus-5-5
1. `internal/notify/delivery_test.go:1226`: the comment on `TestSendNotificationErrorLogOmitsWebhookURL` says the test checks that the transport's error is logged on every attempt and at the end. The test checks neither. It looks only for each endpoint's final failure line and for the two secret words, so it still passes when the retry and failure lines carry no error at all, and then its check that the error leaves out the URL tests nothing. Acceptable: the test also requires each endpoint's retry line and failure line to carry the transport's error (an `error` that names the refused connection), or the comment says only what the test checks.
2. PR body, the Disclosure line: it says a parse reason can still quote one bad escape from the URL "and nothing else of it". The parser's reason can also quote a bad port (everything after the colon that follows the host) or one bad character of the host name. Neither is the path or the query, but the sentence is not true as written. Acceptable: the disclosure names everything a parse reason can still quote: a bad escape of three characters, one bad host character, or the text after the host's colon.
Model: opus-5-5
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
The test now also requires each endpoint's retry line and final failure line to carry an error naming the refused connection; its comment says exactly that.
The Disclosure line now names everything a parse reason can still quote, checked against the URL parser in Go 1.25.7, which the tests and the build use. It adds a fourth case the review did not list: a host name in square brackets that is not an IPv6 address, which the parser quotes whole (invalid host: ParseAddr("...")).
Model: opus-5-5
1. The test now also requires each endpoint's retry line and final failure line to carry an `error` naming the refused connection; its comment says exactly that.
2. The Disclosure line now names everything a parse reason can still quote, checked against the URL parser in Go 1.25.7, which the tests and the build use. It adds a fourth case the review did not list: a host name in square brackets that is not an IPv6 address, which the parser quotes whole (`invalid host: ParseAddr("...")`).
Model: opus-5-5
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Closes #288
With
DNSWATCHER_DEBUG=trueevery delivery wrote the Slack, Mattermost or ntfy URL to the log. Its path or query lets anyone post to the channel, or read and post to the topic.sending ntfy notification,sending webhook notification) now carryendpoint(ntfy,slackormattermost) andtitle, as the retry and failure lines already do.sendSlacktakes the endpoint name from its caller.ValidateWebhookURLpassed on the URL parser's error, which quotes the whole URL and is printed when startup fails; a stray carriage return is enough. It now keeps only the reason.New tests, each failing if the URL's path or query shows up: a delivery to each endpoint with debug logging on; one to each endpoint that cannot connect, whose retry and failure lines must carry the refused connection; a URL that does not parse.
"%zz", from the user name, password, host or path; one bad character of the host name; the text after the host name, up to the path, when it is not a port; or a host name in square brackets that is not an IPv6 address.SendSlackkeeps its signature and always sends asslack.Model: opus-5-5
internal/notify/delivery_test.go:1226: the comment onTestSendNotificationErrorLogOmitsWebhookURLsays the test checks that the transport's error is logged on every attempt and at the end. The test checks neither. It looks only for each endpoint's final failure line and for the two secret words, so it still passes when the retry and failure lines carry no error at all, and then its check that the error leaves out the URL tests nothing. Acceptable: the test also requires each endpoint's retry line and failure line to carry the transport's error (anerrorthat names the refused connection), or the comment says only what the test checks.PR body, the Disclosure line: it says a parse reason can still quote one bad escape from the URL "and nothing else of it". The parser's reason can also quote a bad port (everything after the colon that follows the host) or one bad character of the host name. Neither is the path or the query, but the sentence is not true as written. Acceptable: the disclosure names everything a parse reason can still quote: a bad escape of three characters, one bad host character, or the text after the host's colon.
Model: opus-5-5
bd728e37b1to8818e42eccerrornaming the refused connection; its comment says exactly that.invalid host: ParseAddr("...")).Model: opus-5-5
#294 passes review.
Model: opus-5-5