refactor(core): harden Telegram SDK fetch with marker validation and failure cooldown

- Add `telegramSDKMarker` to reject transparent-proxy block pages and truncated bodies
- Introduce failure cooldown to prevent retry loops when upstream is unreachable
- Move publish/release into defer to avoid permanent latch on panic
- Switch subscription page Cache-Control from public to private
- Add tests for stale-failure loop prevention and non-SDK body rejection
- Improve tg.js script-tag detection and guard assertions in page tests

Author: AppsGanin

internal/core/manager.go

@@ -289,8 +289,8 @@ func New(st *store.Store, sup *xray.Supervisor, opts xray.Options, tls TLSPaths,
 	m.startWebhookWorkers()        // drain the outbound-webhook delivery queue
 	go m.prewarmRoutingTemplates() // warm the routing-template cache so the first
 	//                                  Happ/INCY sub pull after a restart doesn't block
-	go m.refreshTelegramSDK() // fetch telegram-web-app.js so the sub page serves the
-	//                             real SDK (not the shim) from the first Mini App open
+	go m.refreshTelegramSDK() // warm telegram-web-app.js so the first subscription-page
+	//                             view doesn't pay for the fetch inline
 	// NOTE: the initial proxy-pool load is done synchronously by main.go via
 	// SeedProxies() before the first reconcile, so Xray starts once (with proxies)
 	// rather than starting empty and restarting when a background fetch lands.

internal/core/manager_settings.go

@@ -1,6 +1,7 @@
 package core
 
 import (
+	"bytes"
 	"context"
 	"fmt"
 	"regexp"
@@ -631,6 +632,13 @@ const (
 	telegramSDKMaxBytes = 1 << 20         // the wrapper is ~120 KB; 1 MiB is ample headroom
 )
 
+// telegramSDKMarker must appear in a fetched body for it to be cached. netguard
+// already rejects non-200 and enforces https, but a transparent proxy answering 200
+// with an HTML block page would otherwise be cached as JS for a full TTL and served
+// to every user. It also catches a silent truncation at telegramSDKMaxBytes (which
+// returns no error). The real wrapper mentions Telegram.WebApp ~115 times.
+var telegramSDKMarker = []byte("Telegram.WebApp")
+
 // TelegramWebAppSDK returns a server-side cached copy of telegram.org's
 // telegram-web-app.js so the subscription page can serve it from our own
 // (reachable) origin. A fresh copy is returned as-is; a stale one is served
@@ -690,11 +698,16 @@ var telegramSDKFetch = func(ctx context.Context) ([]byte, error) {
 }
 
 // refreshTelegramSDK fetches in the background (startup warm-up and stale refresh).
-// It's a no-op while another fetch is in flight, so the per-request stale trigger
-// can't pile up goroutines.
+// It's a no-op while a fetch is in flight OR while the failure cooldown is armed.
+//
+// That cooldown is what stops a failing upstream from becoming a retry loop: a
+// failed fetch never advances tgSDKAt, so a stale copy stays stale and EVERY
+// subsequent request re-triggers this. Without the guard the panel would dial a
+// blocked telegram.org back-to-back for as long as the page saw traffic — a beacon
+// the decoy story does not cover.
 func (m *Manager) refreshTelegramSDK() {
 	m.tgSDKMu.Lock()
-	if m.tgSDKWait != nil {
+	if m.tgSDKWait != nil || time.Since(m.tgSDKFailAt) < telegramSDKRetryGap {
 		m.tgSDKMu.Unlock()
 		return
 	}
@@ -706,20 +719,32 @@ func (m *Manager) refreshTelegramSDK() {
 // fetchTelegramSDK performs the upstream GET and publishes the result, then releases
 // everyone waiting on it. The caller must have claimed the fetch by setting
 // tgSDKWait. A failed fetch keeps any previous cached copy and stamps tgSDKFailAt so
-// cold readers stop blocking for a while.
+// readers stop blocking for a while.
+//
+// Publish/release runs in a defer so a panic anywhere in the fetch stack can't latch
+// tgSDKWait non-nil forever — that would permanently disable refreshes AND make every
+// later reader wait out the full budget as a rider on a fetch that never completes.
 func (m *Manager) fetchTelegramSDK() {
+	var (
+		b   []byte
+		err error
+	)
+	defer func() {
+		m.tgSDKMu.Lock()
+		if err == nil && bytes.Contains(b, telegramSDKMarker) {
+			m.tgSDKBody, m.tgSDKAt = b, time.Now()
+			m.tgSDKFailAt = time.Time{}
+		} else {
+			m.tgSDKFailAt = time.Now()
+		}
+		if m.tgSDKWait != nil {
+			close(m.tgSDKWait) // wake the riders; they re-read the cache
+			m.tgSDKWait = nil
+		}
+		m.tgSDKMu.Unlock()
+	}()
+
 	ctx, cancel := context.WithTimeout(context.Background(), telegramSDKBudget)
 	defer cancel()
-	b, err := telegramSDKFetch(ctx)
-
-	m.tgSDKMu.Lock()
-	if err == nil && len(b) > 0 {
-		m.tgSDKBody, m.tgSDKAt = b, time.Now()
-		m.tgSDKFailAt = time.Time{}
-	} else {
-		m.tgSDKFailAt = time.Now()
-	}
-	close(m.tgSDKWait) // wake the riders; they re-read the cache
-	m.tgSDKWait = nil
-	m.tgSDKMu.Unlock()
+	b, err = telegramSDKFetch(ctx)
 }

internal/core/manager_tgsdk_test.go

@@ -9,6 +9,14 @@ import (
 	"time"
 )
 
+// Stub bodies must carry telegramSDKMarker — fetchTelegramSDK rejects anything that
+// doesn't look like the real wrapper, so a marker-less fixture would be discarded and
+// every test would read as a fetch failure.
+const (
+	fakeSDK      = "// WebView\nwindow.Telegram.WebApp = {platform:'test'};"
+	fakeSDKFresh = "// WebView fresh\nwindow.Telegram.WebApp = {platform:'test2'};"
+)
+
 // stubTelegramSDKFetch swaps the upstream GET for the duration of a test and
 // reports how many times it was called.
 func stubTelegramSDKFetch(t *testing.T, fn func(ctx context.Context) ([]byte, error)) *atomic.Int32 {
@@ -44,25 +52,41 @@ func waitTelegramSDKIdle(t *testing.T, m *Manager) {
 	}
 }
 
+// waitTelegramSDKCalls waits until the stub has been entered `want` times and the
+// resulting fetch has finished. Needed because a background refresh is spawned with
+// `go`: waiting only for "no fetch in flight" can't tell "not scheduled yet" from
+// "already done", so the test would race ahead and observe zero calls.
+func waitTelegramSDKCalls(t *testing.T, m *Manager, calls *atomic.Int32, want int32) {
+	t.Helper()
+	deadline := time.Now().Add(5 * time.Second)
+	for calls.Load() < want {
+		if time.Now().After(deadline) {
+			t.Fatalf("upstream called %d times after 5s, want %d", calls.Load(), want)
+		}
+		time.Sleep(5 * time.Millisecond)
+	}
+	waitTelegramSDKIdle(t, m)
+}
+
 // A cold cache must fetch INLINE and hand the real body to the very first caller —
 // that's the whole point of the cold path (an empty file would silently disable the
 // Mini App bridge for whoever loads the page first).
 func TestTelegramSDKColdFetchesInline(t *testing.T) {
 	calls := stubTelegramSDKFetch(t, func(context.Context) ([]byte, error) {
-		return []byte("// WebView"), nil
+		return []byte(fakeSDK), nil
 	})
 	m := &Manager{}
 
 	body, ok := m.TelegramWebAppSDK()
-	if !ok || string(body) != "// WebView" {
+	if !ok || string(body) != fakeSDK {
 		t.Fatalf("cold read: got (%q, %v), want the fetched body", body, ok)
 	}
 	if got := calls.Load(); got != 1 {
 		t.Fatalf("upstream called %d times, want 1", got)
 	}
 
 	// Second read is served from cache — no new fetch.
-	if body, ok = m.TelegramWebAppSDK(); !ok || string(body) != "// WebView" {
+	if body, ok = m.TelegramWebAppSDK(); !ok || string(body) != fakeSDK {
 		t.Fatalf("warm read: got (%q, %v)", body, ok)
 	}
 	if got := calls.Load(); got != 1 {
@@ -115,7 +139,7 @@ func TestTelegramSDKConcurrentColdSingleflight(t *testing.T) {
 	release := make(chan struct{})
 	calls := stubTelegramSDKFetch(t, func(context.Context) ([]byte, error) {
 		<-release // hold the fetch open so every goroutine piles up behind it
-		return []byte("// WebView"), nil
+		return []byte(fakeSDK), nil
 	})
 	m := &Manager{}
 
@@ -127,7 +151,7 @@ func TestTelegramSDKConcurrentColdSingleflight(t *testing.T) {
 		go func() {
 			defer wg.Done()
 			body, ok := m.TelegramWebAppSDK()
-			got[i] = ok && string(body) == "// WebView"
+			got[i] = ok && string(body) == fakeSDK
 		}()
 	}
 	time.Sleep(50 * time.Millisecond) // let them all reach the cold path
@@ -144,6 +168,59 @@ func TestTelegramSDKConcurrentColdSingleflight(t *testing.T) {
 	}
 }
 
+// A stale copy plus a failing upstream must NOT become a retry loop. A failed fetch
+// never advances tgSDKAt, so the copy stays stale and every request re-triggers the
+// refresh — without the cooldown guard the panel dials a blocked telegram.org
+// back-to-back for as long as the page sees traffic.
+func TestTelegramSDKStaleFailureDoesNotLoop(t *testing.T) {
+	calls := stubTelegramSDKFetch(t, func(context.Context) ([]byte, error) {
+		return nil, errors.New("dial tcp: i/o timeout")
+	})
+	m := &Manager{}
+	m.tgSDKBody = []byte("old")
+	m.tgSDKAt = time.Now().Add(-telegramSDKTTL - time.Minute) // stale
+
+	// First read kicks off a refresh, which fails and arms the cooldown.
+	if body, ok := m.TelegramWebAppSDK(); !ok || string(body) != "old" {
+		t.Fatalf("stale read: got (%q, %v), want the old body", body, ok)
+	}
+	waitTelegramSDKCalls(t, m, calls, 1)
+
+	// Hammer it: still stale, still failing — the cooldown must absorb every one.
+	for range 50 {
+		if body, ok := m.TelegramWebAppSDK(); !ok || string(body) != "old" {
+			t.Fatalf("stale read regressed: got (%q, %v)", body, ok)
+		}
+	}
+	waitTelegramSDKIdle(t, m)
+	if got := calls.Load(); got != 1 {
+		t.Fatalf("stale+failing upstream looped: %d upstream calls after 51 reads, want 1", got)
+	}
+}
+
+// A 200 that isn't actually the SDK (a transparent-proxy block page, or a body
+// truncated at the size cap — which returns no error) must not be cached: it would
+// be served as JS to every user for a full TTL.
+func TestTelegramSDKRejectsNonSDKBody(t *testing.T) {
+	stubTelegramSDKFetch(t, func(context.Context) ([]byte, error) {
+		return []byte("<html><body>Access denied</body></html>"), nil
+	})
+	m := &Manager{}
+
+	if body, ok := m.TelegramWebAppSDK(); ok || body != nil {
+		t.Fatalf("garbage body was accepted: got (%q, %v), want (nil, false)", body, ok)
+	}
+	m.tgSDKMu.Lock()
+	cached, failed := m.tgSDKBody, !m.tgSDKFailAt.IsZero()
+	m.tgSDKMu.Unlock()
+	if cached != nil {
+		t.Errorf("garbage body got cached: %q", cached)
+	}
+	if !failed {
+		t.Error("a rejected body must arm the failure cooldown like any other failure")
+	}
+}
+
 // A stale copy is served immediately (never blocking the page) while the refresh
 // happens behind it.
 func TestTelegramSDKStaleServedImmediately(t *testing.T) {
@@ -152,7 +229,7 @@ func TestTelegramSDKStaleServedImmediately(t *testing.T) {
 	stubTelegramSDKFetch(t, func(context.Context) ([]byte, error) {
 		once.Do(func() { close(started) })
 		<-block
-		return []byte("fresh"), nil
+		return []byte(fakeSDKFresh), nil
 	})
 	m := &Manager{}
 	m.tgSDKBody = []byte("old")
@@ -180,7 +257,7 @@ func TestTelegramSDKStaleServedImmediately(t *testing.T) {
 	waitTelegramSDKIdle(t, m) // let it land before cleanup restores the stub
 
 	// ...and the refresh it kicked off replaced the stale copy.
-	if body, ok := m.TelegramWebAppSDK(); !ok || string(body) != "fresh" {
+	if body, ok := m.TelegramWebAppSDK(); !ok || string(body) != fakeSDKFresh {
 		t.Fatalf("after refresh: got (%q, %v), want the fresh body", body, ok)
 	}
 }

internal/server/subscription.go

@@ -122,16 +122,20 @@ func handleSub(rt *Router, w http.ResponseWriter, r *http.Request, rest string)
 		// the page never loads it straight from telegram.org (blocked in Russia — a
 		// direct <script> there hangs the page until the connection times out).
 		//
-		// A cold/unreachable cache serves an EMPTY body rather than blocking on an
-		// inline fetch — never hanging the page is the whole point of this route. The
-		// page degrades cleanly: window.Telegram stays undefined, so it treats itself
-		// as a plain browser (INTG=false) exactly as it does today when telegram.org
-		// is blocked. no-store on that empty reply so the client picks up the real SDK
-		// as soon as the background fetch lands.
+		// A COLD cache fetches inline (bounded by the manager's budget) so the first
+		// visitor still gets the real SDK; if telegram.org is unreachable this serves
+		// an EMPTY body instead. The page degrades cleanly on empty: window.Telegram
+		// stays undefined, so it treats itself as a plain browser (INTG=false) exactly
+		// as it did when telegram.org was loaded directly and blocked.
+		//
+		// private, not public: the body is the same non-secret SDK for everyone, but
+		// the URL embeds the user's sub token — no reason to invite shared proxies to
+		// key a cache entry (and a log line) on it. no-store on the empty reply so a
+		// client doesn't pin the miss.
 		js, ok := rt.mgr.TelegramWebAppSDK()
 		w.Header().Set("Content-Type", "text/javascript; charset=utf-8")
 		if ok {
-			w.Header().Set("Cache-Control", "public, max-age=3600")
+			w.Header().Set("Cache-Control", "private, max-age=3600")
 		} else {
 			w.Header().Set("Cache-Control", "no-store")
 		}

internal/sub/page_tgjs_test.go

@@ -1,6 +1,7 @@
 package sub
 
 import (
+	"regexp"
 	"strings"
 	"testing"
 
@@ -25,10 +26,17 @@ func TestPageServesTelegramSDKLocally(t *testing.T) {
 	if !strings.Contains(s, `/tg.js"></script>`) {
 		t.Error("page missing the same-origin <script src=.../tg.js>")
 	}
-	// It must stay a plain blocking script: the inline script at the bottom reads
-	// window.Telegram.WebApp synchronously, so defer/async would break the ordering.
-	if strings.Contains(s, "/tg.js\" defer") || strings.Contains(s, "/tg.js\" async") {
-		t.Error("tg.js must load synchronously, not deferred/async")
+	// It must stay a plain BLOCKING script: the inline script at the bottom reads
+	// window.Telegram.WebApp synchronously, so defer/async would leave it undefined
+	// and silently kill Mini App deep-link routing. Match the whole tag — checking
+	// only for `tg.js" defer` misses `<script async src=...>`, where the attribute
+	// comes first.
+	tag := regexp.MustCompile(`<script[^>]*\btg\.js\b[^>]*>`).FindString(s)
+	if tag == "" {
+		t.Fatal("no <script> tag for tg.js found")
+	}
+	if strings.Contains(tag, "async") || strings.Contains(tag, "defer") {
+		t.Errorf("tg.js must load synchronously, got %q", tag)
 	}
 }
 
@@ -46,10 +54,18 @@ func TestPageToleratesMissingSDK(t *testing.T) {
 	s := string(html)
 	// The guarded read + the INTG flag derived from it are what make an empty SDK
 	// degrade to "plain browser" instead of a ReferenceError.
-	if !strings.Contains(s, "window.Telegram && window.Telegram.WebApp") {
-		t.Error("page must read window.Telegram defensively (empty /tg.js is a valid state)")
+	const guard = "var TG = (window.Telegram && window.Telegram.WebApp) || null;"
+	if !strings.Contains(s, guard) {
+		t.Fatalf("page must read window.Telegram defensively (empty /tg.js is a valid state); want %q", guard)
 	}
 	if !strings.Contains(s, "var INTG") {
 		t.Error("page missing the INTG in-Telegram flag")
 	}
+	// Every other touch of the SDK must go through TG/INTG. A direct
+	// `Telegram.WebApp.…` anywhere else throws on an empty /tg.js and takes the whole
+	// page down with it, so assert the guarded read is the ONLY one. (Static check:
+	// it can't catch an unguarded `TG.foo()`, which the INTG branches cover.)
+	if rest := strings.Replace(s, guard, "", 1); strings.Contains(rest, "Telegram.WebApp") {
+		t.Error("unguarded Telegram.WebApp access outside the guarded read — would throw when /tg.js is empty")
+	}
 }