telegram: a 200 that is not the bot API envelope is not a send (V-615)

The sink parsed the response, and when the body did not unmarshal it fell through to the status check and returned nil on any 2xx. This box reaches telegram through a relay, and a relay that is up but cannot reach api.telegram.org answers 200 with a page of its own. That read as delivered: the dispatcher wrote a 'sent' outbox row and MarkSent restarted the repeat clock, so a sev4 alarm nobody received went quiet for a full interval. Only ok=true is a send now. The response cap moves from 4096 to 64KiB, because a truncated body no longer parses and would read as a failure, and error lines carry a 200-byte snippet instead of the relay's whole page. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-08-06 04:36:13 +04:00
parent 0b1efe4911
commit 8d20efcfbb
2 changed files with 91 additions and 3 deletions
@@ -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())