From 52f14cc5b536a92b318c19a3b187a37033e345b4 Mon Sep 17 00:00:00 2001
From: Moe Hamade <69801237+moehamade@users.noreply.github.com>
Date: Sun, 9 Aug 2026 15:16:51 +0300
Subject: [PATCH] fix(apk): stop a reset header alone from marking a 403 rate
 limited

GitHub sends X-RateLimit-Reset on every REST response, an ordinary 403
included, and it always points at the current window. Feeding it through
retryAtMillis() therefore produced a non-null deadline for any 403, and
the classifier accepted that as proof of a limit. A permissions failure
with the quota untouched came back as RateLimited, so the caller persisted
a cooldown on that route and served stale metadata until a reset window
the failure had nothing to do with.

Classification now looks only at signals that actually mean this request
was the one refused: a spent quota, or an explicit Retry-After. Nothing
real is lost, because GitHub marks a primary limit with
X-RateLimit-Remaining: 0 and a secondary limit with Retry-After. The reset
header keeps its job of supplying the deadline once a limit is
established some other way.

ApkDownloadSourceTest already claimed this contract - its name is "403 is
only treated as a limit when response headers say so" - but its
permissions case passed no reset header at all, which is the one input
that hides the bug. Adds the case it was missing, which fails without this
change, and pins the secondary-limit path so tightening the reset header
cannot blind the client to a Retry-After.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---
 .../bitchat/android/util/ApkDownloadSource.kt | 11 ++++-
 .../android/util/ApkDownloadSourceTest.kt     | 41 +++++++++++++++++++
 2 files changed, 51 insertions(+), 1 deletion(-)

diff --git a/app/src/main/java/com/bitchat/android/util/ApkDownloadSource.kt b/app/src/main/java/com/bitchat/android/util/ApkDownloadSource.kt
index 73f168ba..226e6842 100644
--- a/app/src/main/java/com/bitchat/android/util/ApkDownloadSource.kt
+++ b/app/src/main/java/com/bitchat/android/util/ApkDownloadSource.kt
@@ -150,8 +150,17 @@ internal object ApkDownloadHttpErrors {
             rateLimitResetEpochSeconds = rateLimitResetEpochSeconds,
             nowMillis = nowMillis
         )
+        // X-RateLimit-Reset rides on every GitHub response, an ordinary 403 included, so it
+        // cannot tell an exhausted quota from a permissions failure. Only a spent quota or an
+        // explicit Retry-After says this request was the one that got limited. The reset header
+        // still supplies the deadline below, once being limited is established some other way.
+        val retryAfterMillis = retryAtMillis(
+            retryAfter = retryAfter,
+            rateLimitResetEpochSeconds = null,
+            nowMillis = nowMillis
+        )
         val rateLimited = code == 429 ||
-            (code == 403 && (rateLimitRemaining?.trim() == "0" || retryAt != null))
+            (code == 403 && (rateLimitRemaining?.trim() == "0" || retryAfterMillis != null))
 
         if (rateLimited) {
             return ApkDownloadException(
diff --git a/app/src/test/kotlin/com/bitchat/android/util/ApkDownloadSourceTest.kt b/app/src/test/kotlin/com/bitchat/android/util/ApkDownloadSourceTest.kt
index 4d6702d6..e14a968d 100644
--- a/app/src/test/kotlin/com/bitchat/android/util/ApkDownloadSourceTest.kt
+++ b/app/src/test/kotlin/com/bitchat/android/util/ApkDownloadSourceTest.kt
@@ -101,6 +101,47 @@ class ApkDownloadSourceTest {
         assertEquals(ApkDownloadFailureReason.RateLimited, quotaFailure.reason)
     }
 
+    @Test
+    fun `a reset header alone does not make a 403 a rate limit`() {
+        // GitHub sends X-RateLimit-Reset on every response, so a permissions failure carries one
+        // while the quota is untouched. Reading it as a limit would park the route in a cooldown
+        // and serve stale metadata until a window the failure has nothing to do with.
+        val failure = ApkDownloadHttpErrors.fromResponse(
+            source = source,
+            code = 403,
+            responseMessage = "Forbidden",
+            retryAfter = null,
+            rateLimitRemaining = "4999",
+            rateLimitResetEpochSeconds = (now / 1000L + 1_800L).toString(),
+            nowMillis = now
+        )
+
+        assertEquals(ApkDownloadFailureReason.HttpFailure, failure.reason)
+        assertNull(failure.retryAtMillis)
+        assertEquals(
+            listOf(source.displayName, "403", "Forbidden"),
+            failure.messageArgs
+        )
+    }
+
+    @Test
+    fun `a secondary limit is still caught by its Retry-After`() {
+        // The quota is intact, so only Retry-After marks this one. It has to keep working, or
+        // tightening the reset-header case would blind the client to secondary limits.
+        val failure = ApkDownloadHttpErrors.fromResponse(
+            source = source,
+            code = 403,
+            responseMessage = "Forbidden",
+            retryAfter = "90",
+            rateLimitRemaining = "4999",
+            rateLimitResetEpochSeconds = (now / 1000L + 1_800L).toString(),
+            nowMillis = now
+        )
+
+        assertEquals(ApkDownloadFailureReason.RateLimited, failure.reason)
+        assertEquals(now + 90_000L, failure.retryAtMillis)
+    }
+
     @Test
     fun `invalid or overflowing retry headers never crash error mapping`() {
         assertNull(