Merge the telegram sweep: a 200 that is not the envelope is not a send (#258)

The sink raised an error only when the body parsed AND ok was false. An
unparseable body skipped the check entirely and fell through to the 2xx
test, so any 200 carrying something other than the bot API envelope
returned nil. This box reaches api.telegram.org through a relay, and a
relay that is up but cannot reach telegram answers 200 with an HTML page
of its own.

The consequences compound upward. DispatchNudge writes a DeliverySent
outbox row and Ack.MarkSent restarts the repeat clock, so a sev4 alarm
nobody received goes quiet for a full repeat interval rather than
retrying on the next tick. Only ok:true counts as a send now.

The body cap moves to 64KiB, because under the new rule a truncated
envelope stops parsing and would turn a real send into a false failure.
Error lines carry a 200-byte snippet rather than the relay's whole page.

The rest of internal/delivery is clean, including the double-send path
and the redaction that closed the 2026-08-01 log leak.

(V-615)
This commit is contained in:
2026-08-06 04:41:32 +04:00
2 changed files with 91 additions and 3 deletions
+31 -3
View File
@@ -172,21 +172,49 @@ func (s *Sink) Send(ctx context.Context, d delivery.Sendable) error {
return fmt.Errorf("telegramsink: sendMessage: %w", s.redact(err))
}
defer resp.Body.Close()
rb, _ := io.ReadAll(io.LimitReader(resp.Body, 4096))
rb, _ := io.ReadAll(io.LimitReader(resp.Body, maxRespBytes))
// telegram returns 200 with ok=true on success; non-2xx with ok=false +
// error_code + description on failure. parse the body either way so a 200
// with ok=false (shouldn't happen, but the API reserves that) still surfaces.
var tr telegramResp
if jsonErr := json.Unmarshal(rb, &tr); jsonErr == nil && !tr.Ok {
jsonErr := json.Unmarshal(rb, &tr)
if jsonErr == nil && !tr.Ok {
return fmt.Errorf("telegramsink: telegram returned error %d: %s", tr.ErrorCode, strings.TrimSpace(tr.Description))
}
if resp.StatusCode/100 != 2 {
return fmt.Errorf("telegramsink: telegram returned %d: %s", resp.StatusCode, strings.TrimSpace(string(rb)))
return fmt.Errorf("telegramsink: telegram returned %d: %s", resp.StatusCode, snippet(rb))
}
// A 2xx whose body is not the bot API's envelope did not come from the bot
// API. The normal path here is the relay: this box reaches telegram through
// an HTTP/SOCKS5 proxy, and a proxy that is up but cannot reach
// api.telegram.org answers 200 with an HTML page of its own. Reading that as
// a delivered message is the worst outcome the sink has — the dispatcher
// writes a 'sent' outbox row, MarkSent restarts the repeat clock, and the
// sev4 alarm that never arrived goes quiet for a whole interval. Only
// ok=true is a send.
if jsonErr != nil {
return fmt.Errorf("telegramsink: telegram returned %d with a body that is not the bot API envelope (not a confirmed send): %s", resp.StatusCode, snippet(rb))
}
return nil
}
// maxRespBytes caps the response read — the body is wire-controlled and the
// relay in front of it is not telegram. It is far above any sendMessage
// envelope (a few hundred bytes; the result echoes one short away message),
// because a truncated body no longer parses and now reads as a failed send.
const maxRespBytes = 64 << 10
// snippet trims a response body down to something an error line can carry. A
// relay's HTML page is measured in kilobytes and none of it belongs in the log.
func snippet(rb []byte) string {
s := strings.TrimSpace(string(rb))
if len(s) > 200 {
return s[:200] + "…"
}
return s
}
// sendMessageURL — the bot API path. the token is in the URL path
// (https://api.telegram.org/bot<token>/sendMessage); telegram does not accept
// it anywhere else. the URL is built per-send from the resolved base and never
@@ -291,6 +291,66 @@ func TestSendReturnsErrorOnTelegramError(t *testing.T) {
}
}
// A relay that is up but cannot reach api.telegram.org answers 200 with a page
// of its own. That is not a delivered message, and calling it one silences a
// sev4 alarm for a full repeat interval.
func TestSendRefusesA200ThatIsNotTheBotAPIEnvelope(t *testing.T) {
rs := newRecordingServer(t, http.StatusOK, `<html><body>proxy: upstream unreachable</body></html>`)
srv := httptest.NewServer(rs.handler())
defer srv.Close()
sink, _ := New(sinkCfg(srv.URL))
err := sink.Send(context.Background(), nudgeSendable(loop.Sev4, "down"))
if err == nil {
t.Fatal("want error on a 200 that is not the bot API envelope")
}
if !strings.Contains(err.Error(), "not a confirmed send") {
t.Fatalf("error should say the send is unconfirmed, got: %v", err)
}
}
// The success path must stay a success: ok=true on 200 is a send.
func TestSendAcceptsOkTrue(t *testing.T) {
rs := newRecordingServer(t, http.StatusOK, `{"ok":true,"result":{"message_id":7}}`)
srv := httptest.NewServer(rs.handler())
defer srv.Close()
sink, _ := New(sinkCfg(srv.URL))
if err := sink.Send(context.Background(), nudgeSendable(loop.Sev4, "down")); err != nil {
t.Fatalf("want success on ok=true, got: %v", err)
}
}
// A body long enough to have been truncated by the old 4096-byte cap still
// parses, so a real send is not reported as a failure.
func TestSendAcceptsAnOversizedButValidEnvelope(t *testing.T) {
rs := newRecordingServer(t, http.StatusOK,
`{"ok":true,"result":{"message_id":7,"text":"`+strings.Repeat("x", 8000)+`"}}`)
srv := httptest.NewServer(rs.handler())
defer srv.Close()
sink, _ := New(sinkCfg(srv.URL))
if err := sink.Send(context.Background(), nudgeSendable(loop.Sev4, "down")); err != nil {
t.Fatalf("want success on a large ok=true envelope, got: %v", err)
}
}
// The error line carries a snippet, not the relay's whole page.
func TestSendErrorDoesNotCarryTheWholeBody(t *testing.T) {
rs := newRecordingServer(t, http.StatusBadGateway, strings.Repeat("z", 5000))
srv := httptest.NewServer(rs.handler())
defer srv.Close()
sink, _ := New(sinkCfg(srv.URL))
err := sink.Send(context.Background(), nudgeSendable(loop.Sev4, "down"))
if err == nil {
t.Fatal("want error on 502")
}
if len(err.Error()) > 500 {
t.Fatalf("error line should be trimmed, got %d bytes", len(err.Error()))
}
}
func TestSendReturnsErrorOnNon2xx(t *testing.T) {
rs := newRecordingServer(t, http.StatusBadGateway, "bad gateway")
srv := httptest.NewServer(rs.handler())