fix(telegram): never expose bot token in avatar URL

tgAPI.Avatar returned a URL with the bot token embedded in its path:

    https://api.telegram.org/file/bot{TOKEN}/photos/file_X.jpg

The token is a bearer credential for the entire bot API. The URL flowed
into User.Picture and from there:

* Into avatar.Proxy.Put debug logs ("[DEBUG] saved avatar from <url>"
  and the corresponding load-failure line) regardless of whether avatar
  saving succeeded.
* Into the JWT claims and the user JSON returned to the browser when
  no AvatarSaver was configured (User.Picture is in the User struct).

Either path leaks the bot token to anyone with log access, anyone who
can read the JWT (the user themselves on the device, plus anyone
intercepting browser/devtools), or any third-party observability stack.

Two-part fix in v1 and v2:

1. avatar/avatar.go: redact the URL in Put's two debug log lines via a
   new redactAvatarURL helper (hostname only). Add Proxy.PutContent so
   pre-fetched bytes can be saved without the URL-fetch round trip.
2. provider/telegram.go: in processUpdates, never assign the bot URL
   to User.Picture. Pass it to a new saveTelegramAvatar method that
   fetches the bytes server-side and stores them via the new content-
   saver interface (avatar.Proxy implements it). The call returns a
   clean local proxy URL or "" — whatever lands in Picture is safe to
   log and to send to the client.

A graceful fallback path warns and drops the avatar when the
configured AvatarSaver does not implement PutContent (custom external
implementations) — never exposes the token to satisfy the avatar
feature.

Tests in both modules:

* TestSaveTelegramAvatar_BotTokenNeverLogged — unit-level table for
  the helper covering the success, fallback-without-PutContent and
  empty-URL paths.
* TestTelegramProcessUpdates_BotTokenNeverInUserPicture — regression
  test for the property: drive processUpdates with a mock that returns
  a URL containing a bot-token marker; assert the marker never lands
  in user.Picture and never appears in any captured log line.
  Reverting the saveTelegramAvatar redirection makes this test fail
  with a clear assertion message.

Author: paskal

README.md

@@ -377,6 +377,29 @@ go func() {
 service.AddCustomHandler(&telegram)
 ```
 
+#### Avatar handling
+
+Telegram's file API requires the bot token in the URL path
+(`https://api.telegram.org/file/bot<TOKEN>/...`), so the URL is a bearer
+credential and must never reach the JWT, the user JSON, or any debug log.
+The provider fetches the avatar bytes server-side and stores them via
+`AvatarSaver`, returning a clean local proxy URL.
+
+This requires `AvatarSaver` to support direct content saving.
+`avatar.Proxy` (the default returned by `service.AvatarProxy()`) does -- no
+extra setup needed. **Custom `AvatarSaver` implementations** that don't
+implement `PutContent(userID string, content io.Reader) (string, error)`
+will see an empty `User.Picture` for Telegram logins, and the avatar proxy
+will fall back to identicon. To preserve avatars with a custom saver,
+implement that method alongside `Put`.
+
+> **Behaviour change for custom `AvatarSaver`:** before this change, custom
+> savers received the raw Telegram file URL (which included the bot token)
+> via `Put`. After this change they no longer do, because that URL is a
+> bearer credential. Implementations that relied on receiving and refetching
+> that URL must now implement `PutContent` to keep saving Telegram avatars,
+> or accept that Telegram users get identicons.
+
 Now all your users have to do is click one of the following links and press **start**
 `tg://resolve?domain=<botname>&start=<token>` or `https://t.me/<botname>/?start=<token>`
 

avatar/avatar.go

@@ -11,6 +11,7 @@ import (
 	"image/png"
 	"io"
 	"net/http"
+	"net/url"
 	"strconv"
 	"strings"
 	"time"
@@ -26,6 +27,12 @@ import (
 // http.sniffLen is 512 bytes which is how much we need to read to detect content type
 const sniffLen = 512
 
+// maxAvatarFetchSize bounds the bytes read from a remote avatar URL. 10 MiB is
+// generous for any reasonable avatar (Telegram caps photo at 5 MiB; Gravatar is
+// much smaller); the cap protects Proxy.Put against an upstream sending an
+// unbounded body that would exhaust process memory inside resize.
+const maxAvatarFetchSize = 10 << 20
+
 // Proxy provides http handler for avatars from avatar.Store
 // On user login token will call Put and it will retrieve and save picture locally.
 type Proxy struct {
@@ -61,7 +68,7 @@ func (p *Proxy) Put(u token.User, client *http.Client) (avatarURL string, err er
 
 	body, err := p.load(u.Picture, client)
 	if err != nil {
-		p.Logf("[DEBUG] failed to fetch avatar from the orig %s, %v", u.Picture, err)
+		p.Logf("[DEBUG] failed to fetch avatar from the orig %s, %v", redactAvatarURL(u.Picture), err)
 		return genIdenticon(u.ID)
 	}
 
@@ -76,10 +83,36 @@ func (p *Proxy) Put(u token.User, client *http.Client) (avatarURL string, err er
 		return "", err
 	}
 
-	p.Logf("[DEBUG] saved avatar from %s to %s, user %q", u.Picture, avatarID, u.Name)
+	p.Logf("[DEBUG] saved avatar from %s to %s, user %q", redactAvatarURL(u.Picture), avatarID, u.Name)
+	return p.URL + p.RoutePath + "/" + avatarID, nil
+}
+
+// PutContent stores already-fetched avatar bytes via the underlying Store and returns
+// the proxied URL. It exists so providers that authenticate with credentials embedded
+// in the upstream URL (e.g. Telegram bot file API: /file/bot{TOKEN}/...) can fetch the
+// content themselves and avoid exposing the credential to Put's URL-fetching path —
+// where it would land in u.Picture, debug logs, and the user JSON returned to clients.
+func (p *Proxy) PutContent(userID string, content io.Reader) (avatarURL string, err error) {
+	avatarID, err := p.Store.Put(userID, p.resize(content, p.ResizeLimit))
+	if err != nil {
+		return "", err
+	}
+	p.Logf("[DEBUG] saved avatar bytes to %s, user %q", avatarID, userID)
 	return p.URL + p.RoutePath + "/" + avatarID, nil
 }
 
+// redactAvatarURL returns the hostname only, dropping scheme, userinfo, path,
+// query and fragment. This is enough to keep avatar URLs identifiable in logs
+// while ensuring credentials carried in any of those parts (e.g. Telegram bot
+// tokens, time-limited signed-URL tokens, basic-auth in userinfo) don't reach
+// log destinations. On parse failure a sentinel is returned.
+func redactAvatarURL(raw string) string {
+	if u, err := url.Parse(raw); err == nil && u.Hostname() != "" {
+		return u.Hostname()
+	}
+	return "<unparseable>"
+}
+
 // load avatar from remote url and return body. Caller has to close the reader
 func (p *Proxy) load(url string, client *http.Client) (rc io.ReadCloser, err error) {
 	// load avatar from remote location
@@ -98,7 +131,17 @@ func (p *Proxy) load(url string, client *http.Client) (rc io.ReadCloser, err err
 		return nil, fmt.Errorf("failed to get avatar from the orig, status %s", resp.Status)
 	}
 
-	return resp.Body, nil
+	// buffer the body up to the cap to fail fast on oversized inputs.
+	// Reading +1 byte beyond the cap distinguishes "exactly cap" from "too big".
+	body, err := io.ReadAll(io.LimitReader(resp.Body, maxAvatarFetchSize+1))
+	_ = resp.Body.Close()
+	if err != nil {
+		return nil, fmt.Errorf("failed to read avatar body: %w", err)
+	}
+	if int64(len(body)) > maxAvatarFetchSize {
+		return nil, fmt.Errorf("avatar body exceeds %d bytes", maxAvatarFetchSize)
+	}
+	return io.NopCloser(bytes.NewReader(body)), nil
 }
 
 // Handler returns token routes for given provider

avatar/avatar_test.go

@@ -58,6 +58,34 @@ func TestAvatar_Put(t *testing.T) {
 	assert.Equal(t, int64(21), fi.Size())
 }
 
+func TestAvatar_PutContent(t *testing.T) {
+	defer func() { _ = os.RemoveAll("/tmp/avatars.put-content.test/") }()
+	p := Proxy{RoutePath: "/avatar", URL: "http://localhost:8080", Store: NewLocalFS("/tmp/avatars.put-content.test"), L: logger.Std}
+	got, err := p.PutContent("user1", strings.NewReader("png-bytes"))
+	require.NoError(t, err)
+	assert.Equal(t, "http://localhost:8080/avatar/b3daa77b4c04a9551b8781d03191fe098f325e67.image", got)
+	fi, err := os.Stat("/tmp/avatars.put-content.test/30/b3daa77b4c04a9551b8781d03191fe098f325e67.image")
+	require.NoError(t, err)
+	assert.Greater(t, fi.Size(), int64(0))
+}
+
+func TestAvatar_RedactAvatarURL(t *testing.T) {
+	cases := []struct {
+		in, want string
+	}{
+		{in: "https://api.telegram.org/file/botSECRET/photo.jpg", want: "api.telegram.org"},
+		{in: "https://x:y@example.com/path?q=1", want: "example.com"}, // #nosec G101 -- deliberate test fixture for userinfo redaction
+		{in: "", want: "<unparseable>"},
+		{in: "/local/path", want: "<unparseable>"},
+		{in: "://malformed", want: "<unparseable>"},
+	}
+	for _, c := range cases {
+		t.Run(c.in, func(t *testing.T) {
+			assert.Equal(t, c.want, redactAvatarURL(c.in))
+		})
+	}
+}
+
 func TestAvatar_PutIdenticon(t *testing.T) {
 	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		log.Print("request: ", r.URL.Path)
@@ -103,6 +131,31 @@ func TestAvatar_PutFailed(t *testing.T) {
 	assert.Equal(t, int64(992), fi.Size())
 }
 
+func TestAvatar_PutCapsBodySize(t *testing.T) {
+	// upstream that streams indefinitely past the cap; without
+	// the maxAvatarFetchSize check resize would consume all bytes.
+	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+		w.Header().Set("Content-Type", "image/*")
+		w.WriteHeader(http.StatusOK)
+		buf := make([]byte, 64<<10)
+		for i := 0; i < (maxAvatarFetchSize/len(buf))+32; i++ {
+			if _, err := w.Write(buf); err != nil {
+				return
+			}
+		}
+	}))
+	defer ts.Close()
+
+	dir := t.TempDir()
+	p := Proxy{RoutePath: "/avatar", URL: "http://localhost:8080", Store: NewLocalFS(dir), L: logger.NoOp}
+	client := &http.Client{Timeout: 5 * time.Second}
+
+	u := token.User{ID: "user1", Name: "huge avatar", Picture: ts.URL + "/pic.png"}
+	res, err := p.Put(u, client)
+	require.NoError(t, err, "Put falls back to identicon on capped fetch failure")
+	assert.Contains(t, res, "/avatar/", "still returns a proxy URL via identicon fallback")
+}
+
 func TestAvatar_Routes(t *testing.T) {
 
 	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {

provider/oauth2_test.go

@@ -450,3 +450,14 @@ func (m *mockAvatarSaver) Put(u token.User, _ *http.Client) (avatarURL string, e
 	return "http://example.com/fake.png", nil
 
 }
+
+// PutContent satisfies the avatarContentSaver type assertion in telegram.go so
+// the test mock behaves like avatar.Proxy and the bot-token-aware path can be
+// exercised. The body is drained but the saver returns the same canned URL it
+// would for a regular Put, so existing tests asserting on Picture stay valid.
+func (m *mockAvatarSaver) PutContent(_ string, content io.Reader) (avatarURL string, err error) {
+	if _, e := io.Copy(io.Discard, content); e != nil {
+		return "", e
+	}
+	return "http://example.com/ava12345.png", nil
+}

provider/telegram.go

@@ -3,13 +3,16 @@ package provider
 //go:generate moq --out telegram_moq_test.go . TelegramAPI
 
 import (
+	"bytes"
 	"context"
 	"crypto/sha1"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"io"
 	"net/http"
 	neturl "net/url"
+	"regexp"
 	"strings"
 	"sync"
 	"sync/atomic"
@@ -19,6 +22,7 @@ import (
 	"github.com/go-pkgz/rest"
 	"github.com/golang-jwt/jwt"
 
+	"github.com/go-pkgz/auth/avatar"
 	"github.com/go-pkgz/auth/logger"
 	authtoken "github.com/go-pkgz/auth/token"
 )
@@ -186,11 +190,18 @@ func (th *TelegramHandler) processUpdates(ctx context.Context, updates *telegram
 
 		id := th.ProviderName + "_" + authtoken.HashID(sha1.New(), fmt.Sprint(update.Message.Chat.ID))
 
+		// avatarURL embeds the bot token in its path
+		// (https://api.telegram.org/file/bot{TOKEN}/...). Never store it in
+		// User.Picture: it would leak through avatar.Proxy.Put logs and, when
+		// no avatar saver is configured, into the JWT and on to the client.
+		// Fetch the bytes here and hand them to the avatar store directly.
+		picture := th.saveTelegramAvatar(ctx, id, avatarURL)
+
 		authRequest.confirmed = true
 		authRequest.user = &authtoken.User{
 			ID:      id,
 			Name:    update.Message.Chat.Name,
-			Picture: avatarURL,
+			Picture: picture,
 		}
 
 		th.requests.Lock()
@@ -294,10 +305,19 @@ func (th *TelegramHandler) LoginHandler(w http.ResponseWriter, r *http.Request)
 		return
 	}
 
-	u, err := setAvatar(th.AvatarSaver, *authUser, &http.Client{Timeout: 5 * time.Second})
-	if err != nil {
-		rest.SendErrorJSON(w, r, th.L, http.StatusInternalServerError, err, "failed to save avatar to proxy")
-		return
+	// when saveTelegramAvatar already populated Picture with a local proxy
+	// URL, skip the URL-fetching avatar pipeline. Letting setAvatar run
+	// here would have it call Proxy.Put which re-fetches Picture; in
+	// split-DNS / unreachable-internal-Opts.URL deployments that fetch
+	// fails and the identicon fallback would silently overwrite the
+	// stored Telegram bytes with an identicon at the same store path.
+	u := *authUser
+	if u.Picture == "" {
+		u, err = setAvatar(th.AvatarSaver, *authUser, &http.Client{Timeout: 5 * time.Second})
+		if err != nil {
+			rest.SendErrorJSON(w, r, th.L, http.StatusInternalServerError, err, "failed to save avatar to proxy")
+			return
+		}
 	}
 
 	claims := authtoken.Claims{
@@ -455,12 +475,12 @@ func (tg *tgAPI) request(ctx context.Context, method string, data any) error {
 
 		req, err := http.NewRequestWithContext(ctx, "GET", url, http.NoBody)
 		if err != nil {
-			return fmt.Errorf("failed to create request: %w", err)
+			return fmt.Errorf("failed to create request: %w", redactBotURLInErr(err))
 		}
 
 		resp, err := tg.client.Do(req)
 		if err != nil {
-			return fmt.Errorf("failed to send request: %w", err)
+			return fmt.Errorf("failed to send request: %w", redactBotURLInErr(err))
 		}
 		defer resp.Body.Close() //nolint gosec // we don't care about response body
 
@@ -485,3 +505,101 @@ func (tg *tgAPI) parseError(r io.Reader, statusCode int) error {
 	}
 	return fmt.Errorf("unexpected telegram API status code %d, error: %q", statusCode, tgErr.Description)
 }
+
+// avatarContentSaver matches the optional method on AvatarSaver implementations
+// that can store already-fetched bytes (avatar.Proxy provides one). Used by the
+// Telegram provider to avoid passing a bot-token-bearing URL through the
+// URL-fetching avatar pipeline.
+type avatarContentSaver interface {
+	PutContent(userID string, content io.Reader) (string, error)
+}
+
+// saveTelegramAvatar fetches the avatar bytes from a bot-token-bearing Telegram
+// URL and stores them via th.AvatarSaver, returning a clean local proxy URL.
+// The bot URL is consumed entirely inside this function so it never reaches
+// User.Picture, JWT claims, or any debug log of the user object. Returns ""
+// when the avatar cannot be saved (no URL, no compatible saver, or fetch
+// failure) — the caller treats that as "no picture" and the avatar pipeline
+// falls back to identicon as usual.
+func (th *TelegramHandler) saveTelegramAvatar(ctx context.Context, userID, avatarURL string) string {
+	if avatarURL == "" {
+		return ""
+	}
+	// guard against typed-nil *avatar.Proxy. auth.go skips initializing
+	// res.avatarProxy when Opts.AvatarStore is unset, so AvatarSaver can be
+	// a non-nil interface wrapping a nil *avatar.Proxy. The type assertion
+	// below would still succeed (interface satisfaction is structural), but
+	// PutContent on a nil receiver panics on the first p.Store deref.
+	if th.AvatarSaver == nil || th.AvatarSaver == (*avatar.Proxy)(nil) {
+		th.Logf("[WARN] telegram avatar dropped: AvatarSaver is not configured")
+		return ""
+	}
+	saver, ok := th.AvatarSaver.(avatarContentSaver)
+	if !ok {
+		// fallback intentionally drops the picture rather than expose the bot
+		// token; warn so operators can wire a content-aware saver if they want
+		// telegram avatars saved
+		th.Logf("[WARN] telegram avatar dropped: configured AvatarSaver does not support direct content save")
+		return ""
+	}
+	req, err := http.NewRequestWithContext(ctx, http.MethodGet, avatarURL, http.NoBody)
+	if err != nil {
+		th.Logf("[WARN] telegram avatar fetch request build failed: %v", redactBotURLInErr(err))
+		return ""
+	}
+	client := &http.Client{Timeout: 5 * time.Second}
+	resp, err := client.Do(req)
+	if err != nil {
+		th.Logf("[WARN] telegram avatar fetch failed: %v", redactBotURLInErr(err))
+		return ""
+	}
+	defer func() { _ = resp.Body.Close() }()
+	if resp.StatusCode != http.StatusOK {
+		th.Logf("[WARN] telegram avatar fetch returned status %d", resp.StatusCode)
+		return ""
+	}
+	// cap body size to protect PutContent from an unbounded upstream response.
+	// Telegram caps photos at 5 MiB; 10 MiB is generous headroom while still
+	// bounding worst-case memory.
+	body, err := io.ReadAll(io.LimitReader(resp.Body, maxTelegramAvatarSize+1))
+	if err != nil {
+		th.Logf("[WARN] telegram avatar read failed: %v", err)
+		return ""
+	}
+	if int64(len(body)) > maxTelegramAvatarSize {
+		th.Logf("[WARN] telegram avatar dropped: body exceeds %d bytes", maxTelegramAvatarSize)
+		return ""
+	}
+	picture, err := saver.PutContent(userID, bytes.NewReader(body))
+	if err != nil {
+		th.Logf("[WARN] telegram avatar save failed: %v", err)
+		return ""
+	}
+	return picture
+}
+
+const maxTelegramAvatarSize = 10 << 20
+
+// botTokenInURLPath matches the bot-token segment of a Telegram URL anchored
+// between path slashes ("/botTOKEN/..."). The leading and trailing slashes
+// avoid matching unrelated identifiers that happen to start with "bot" (e.g.
+// the username "botFather" appearing elsewhere in a log line). Replacement
+// preserves the slashes via "/bot<redacted>/" to keep surrounding URL
+// structure intact for diagnostics.
+var botTokenInURLPath = regexp.MustCompile(`/bot[A-Za-z0-9:_-]+/`)
+
+// redactBotURLInErr returns the error with any embedded Telegram bot-token
+// segment in URL paths replaced by "bot<redacted>". net/http's *url.Error
+// stringifies as `Op "URL": Err`, so a transport failure on a URL like
+// https://api.telegram.org/file/bot<TOKEN>/... otherwise prints the token
+// verbatim.
+func redactBotURLInErr(err error) error {
+	if err == nil {
+		return nil
+	}
+	redacted := botTokenInURLPath.ReplaceAllString(err.Error(), "/bot<redacted>/")
+	if redacted == err.Error() {
+		return err
+	}
+	return errors.New(redacted)
+}