Skip to content

Commit 80df5aa

Browse files
gustavobertoiclaude
andcommitted
fix(selfupdate): report a rate-limited 403 as rate limiting, not a private repo
GitHub answers an exhausted API quota with a plain 403 Forbidden, which is indistinguishable from a permissions failure unless you read the response headers. Both `self update` and install.sh assumed the permissions case for every 403 and told the user to "set GITHUB_TOKEN if the repo is private" — so a user on a PUBLIC repo who had merely used up the anonymous 60-requests/hour quota went hunting for a permissions problem that did not exist, while the real cause (and the fact that it clears itself in minutes) stayed hidden. The distinguishing signal is X-RateLimit-Remaining: 0, plus Retry-After for secondary limits. apiError now separates four cases: rate limited -> names the limit that was hit, says plainly that it is NOT a permissions problem, gives the reset countdown, and still offers GITHUB_TOKEN as the fix (it raises the cap to 5000/h) 401 -> the token is set but was rejected 404 -> keeps the private-repo hint, which is where it belongs other -> the bare status Before: Github API https://api.github.com/... returned 403 Forbidden (set GITHUB_TOKEN if the repo is private). After: GitHub API rate limit exceeded (60 requests/hour), resets in 10m0s. This is not a permissions problem — the repository is reachable, you have simply used up the anonymous quota for your IP. Set GITHUB_TOKEN (or GH_TOKEN) to raise the limit to 5000 requests/hour: <url> Applied to both the API path (selfupdate.go) and the asset-download path (update.go), since the reported failure hit the asset download first. install.sh cannot read the headers — its api() helper uses `curl -f`, which discards the error body — so it probes /rate_limit (which does not count against the quota) and appends the same clarification when the quota is spent. The reset countdown is omitted rather than rendered when the stamp is in the past or unparseable, so it can never print "resets in -3m0s". Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 461f4fd commit 80df5aa

5 files changed

Lines changed: 221 additions & 3 deletions

File tree

‎install.sh‎

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,22 @@ else
5151
fi
5252
have tar || die "need tar to unpack the release archive"
5353

54+
# rate_limit_note explains an exhausted GitHub API quota.
55+
#
56+
# GitHub answers a spent anonymous quota with 403 Forbidden, which is
57+
# indistinguishable from a permissions failure unless you look at the limit
58+
# itself. Without this, a PUBLIC repo that is merely rate-limited reports as
59+
# "if the repo is private", sending people to hunt a problem that is not there.
60+
# /rate_limit does not itself count against the quota.
61+
rate_limit_note() {
62+
rl="$(dl "https://api.github.com/rate_limit" 2>/dev/null || true)"
63+
case "$rl" in
64+
*'"remaining":0'* | *'"remaining": 0'*)
65+
printf '%s' " The GitHub API rate limit for your IP is exhausted — this is NOT a permissions problem. Set GITHUB_TOKEN to raise the limit to 5000/hour, or wait for the window to reset."
66+
;;
67+
esac
68+
}
69+
5470
# extract_tag pulls the first tag_name out of a GitHub releases JSON payload.
5571
extract_tag() { grep '"tag_name"' | head -n1 | sed -E 's/.*"tag_name":[[:space:]]*"([^"]+)".*/\1/'; }
5672

@@ -77,7 +93,7 @@ if [ -z "$tag" ]; then
7793
# pre-release-only repos and the brief post-publish API propagation window).
7894
tag="$(api "${API}/releases/latest" 2>/dev/null | extract_tag || true)"
7995
[ -n "$tag" ] || tag="$(api "${API}/releases" 2>/dev/null | extract_tag || true)"
80-
[ -n "$tag" ] || die "could not determine the latest release. Pin one with DEVSTACK_VERSION=vX.Y.Z, and if the repo is private set GITHUB_TOKEN."
96+
[ -n "$tag" ] || die "could not determine the latest release. Pin one with DEVSTACK_VERSION=vX.Y.Z, and set GITHUB_TOKEN if the repo is private.$(rate_limit_note)"
8197
fi
8298
# goreleaser strips the leading 'v' from the archive filename's version field.
8399
version="${tag#v}"

‎internal/selfupdate/apierror.go‎

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
package selfupdate
2+
3+
import (
4+
"fmt"
5+
"net/http"
6+
"strconv"
7+
"time"
8+
)
9+
10+
// nowFn is swappable so the reset-countdown wording is testable.
11+
var nowFn = time.Now
12+
13+
// apiError turns a non-200 GitHub response into an error that names the ACTUAL
14+
// cause.
15+
//
16+
// GitHub reports an exhausted rate limit as a plain 403, which is
17+
// indistinguishable from a permissions failure unless you read the headers. The
18+
// previous message assumed the permissions case for every 403 and told the user
19+
// to "set GITHUB_TOKEN if the repo is private" — so a user hitting the
20+
// unauthenticated 60-requests/hour cap on a PUBLIC repo went looking for a
21+
// permissions problem that did not exist.
22+
//
23+
// The distinguishing signal is X-RateLimit-Remaining: 0. Setting a token is
24+
// still the right advice when rate-limited (it raises the cap to 5000/hour), but
25+
// the reason and the wait time matter more than the guess about visibility.
26+
func apiError(url string, resp *http.Response) error {
27+
if isRateLimited(resp) {
28+
limit := resp.Header.Get("X-RateLimit-Limit")
29+
if limit == "" {
30+
limit = "the anonymous"
31+
} else {
32+
limit += " requests/hour"
33+
}
34+
return fmt.Errorf(
35+
"GitHub API rate limit exceeded (%s)%s.\n"+
36+
"This is not a permissions problem — the repository is reachable, you have simply "+
37+
"used up the anonymous quota for your IP.\n"+
38+
"Set GITHUB_TOKEN (or GH_TOKEN) to raise the limit to 5000 requests/hour: %s",
39+
limit, resetHint(resp), url)
40+
}
41+
switch resp.StatusCode {
42+
case http.StatusUnauthorized:
43+
return fmt.Errorf("GitHub API %s returned %s — GITHUB_TOKEN is set but was rejected; "+
44+
"check that it is valid and not expired", url, resp.Status)
45+
case http.StatusNotFound:
46+
return fmt.Errorf("GitHub API %s returned %s (set GITHUB_TOKEN if the repository is private)",
47+
url, resp.Status)
48+
}
49+
return fmt.Errorf("GitHub API %s returned %s", url, resp.Status)
50+
}
51+
52+
// isRateLimited reports whether a response is a rate-limit rejection. GitHub uses
53+
// 403 for the primary limit and 429 for secondary limits; both carry a zeroed
54+
// X-RateLimit-Remaining, and a secondary limit may carry only Retry-After.
55+
func isRateLimited(resp *http.Response) bool {
56+
if resp.StatusCode != http.StatusForbidden && resp.StatusCode != http.StatusTooManyRequests {
57+
return false
58+
}
59+
if resp.Header.Get("X-RateLimit-Remaining") == "0" {
60+
return true
61+
}
62+
return resp.Header.Get("Retry-After") != ""
63+
}
64+
65+
// resetHint renders ", resets in 9m30s" when the response says when the window
66+
// rolls over, and "" when it does not — never a bare or negative duration.
67+
func resetHint(resp *http.Response) string {
68+
if ra := resp.Header.Get("Retry-After"); ra != "" {
69+
if secs, err := strconv.Atoi(ra); err == nil && secs > 0 {
70+
return fmt.Sprintf(", retry in %s", (time.Duration(secs) * time.Second).String())
71+
}
72+
}
73+
reset := resp.Header.Get("X-RateLimit-Reset")
74+
if reset == "" {
75+
return ""
76+
}
77+
epoch, err := strconv.ParseInt(reset, 10, 64)
78+
if err != nil {
79+
return ""
80+
}
81+
d := time.Until(time.Unix(epoch, 0)).Round(time.Second)
82+
if nowFn != nil {
83+
d = time.Unix(epoch, 0).Sub(nowFn()).Round(time.Second)
84+
}
85+
if d <= 0 {
86+
return ""
87+
}
88+
return fmt.Sprintf(", resets in %s", d.String())
89+
}
Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,113 @@
1+
package selfupdate
2+
3+
import (
4+
"net/http"
5+
"strconv"
6+
"strings"
7+
"testing"
8+
"time"
9+
)
10+
11+
func resp(status int, hdr map[string]string) *http.Response {
12+
h := http.Header{}
13+
for k, v := range hdr {
14+
h.Set(k, v)
15+
}
16+
return &http.Response{StatusCode: status, Status: strconv.Itoa(status) + " " + http.StatusText(status), Header: h}
17+
}
18+
19+
// TestRateLimitedForbiddenIsNotReportedAsPermissions is the regression this file
20+
// exists for. A user on a PUBLIC repo exhausted the anonymous 60/hour quota and
21+
// got "set GITHUB_TOKEN if the repo is private", which describes a problem that
22+
// did not exist and hid the one that did.
23+
func TestRateLimitedForbiddenIsNotReportedAsPermissions(t *testing.T) {
24+
// Exactly the headers GitHub returned in that session.
25+
r := resp(http.StatusForbidden, map[string]string{
26+
"X-RateLimit-Limit": "60",
27+
"X-RateLimit-Remaining": "0",
28+
"X-RateLimit-Reset": strconv.FormatInt(time.Now().Add(10*time.Minute).Unix(), 10),
29+
})
30+
err := apiError("https://api.github.com/repos/o/r/releases", r)
31+
msg := err.Error()
32+
33+
if !strings.Contains(msg, "rate limit exceeded") {
34+
t.Errorf("the message must name the real cause, got: %s", msg)
35+
}
36+
if strings.Contains(msg, "if the repository is private") || strings.Contains(msg, "if the repo is private") {
37+
t.Errorf("a rate-limited response must NOT be reported as a permissions problem, got: %s", msg)
38+
}
39+
if !strings.Contains(msg, "60 requests/hour") {
40+
t.Errorf("the message should quote the limit that was hit, got: %s", msg)
41+
}
42+
if !strings.Contains(msg, "GITHUB_TOKEN") {
43+
t.Errorf("the message should still offer the fix, got: %s", msg)
44+
}
45+
if !strings.Contains(msg, "resets in") {
46+
t.Errorf("the message should say when the window rolls over, got: %s", msg)
47+
}
48+
}
49+
50+
func TestSecondaryRateLimitViaRetryAfter(t *testing.T) {
51+
r := resp(http.StatusTooManyRequests, map[string]string{"Retry-After": "45"})
52+
msg := apiError("https://api.github.com/x", r).Error()
53+
if !strings.Contains(msg, "rate limit exceeded") {
54+
t.Errorf("429 with Retry-After should read as rate limiting, got: %s", msg)
55+
}
56+
if !strings.Contains(msg, "retry in 45s") {
57+
t.Errorf("should surface Retry-After, got: %s", msg)
58+
}
59+
}
60+
61+
// TestForbiddenWithQuotaLeftIsNotRateLimit: a 403 that still has quota is a real
62+
// permissions failure and must keep the private-repo hint.
63+
func TestForbiddenWithQuotaLeftIsNotRateLimit(t *testing.T) {
64+
r := resp(http.StatusForbidden, map[string]string{
65+
"X-RateLimit-Limit": "5000",
66+
"X-RateLimit-Remaining": "4999",
67+
})
68+
msg := apiError("https://api.github.com/x", r).Error()
69+
if strings.Contains(msg, "rate limit exceeded") {
70+
t.Errorf("403 with quota remaining is not rate limiting, got: %s", msg)
71+
}
72+
}
73+
74+
func TestNotFoundKeepsThePrivateRepoHint(t *testing.T) {
75+
msg := apiError("https://api.github.com/x", resp(http.StatusNotFound, nil)).Error()
76+
if !strings.Contains(msg, "private") {
77+
t.Errorf("404 is where the private-repo hint belongs, got: %s", msg)
78+
}
79+
}
80+
81+
func TestUnauthorizedBlamesTheToken(t *testing.T) {
82+
msg := apiError("https://api.github.com/x", resp(http.StatusUnauthorized, nil)).Error()
83+
if !strings.Contains(msg, "rejected") {
84+
t.Errorf("401 means the token is bad, not that the repo is private, got: %s", msg)
85+
}
86+
}
87+
88+
// TestResetHintNeverShowsAStaleOrNegativeDuration: a reset stamp in the past
89+
// would otherwise render as "resets in -3m0s".
90+
func TestResetHintNeverShowsAStaleOrNegativeDuration(t *testing.T) {
91+
r := resp(http.StatusForbidden, map[string]string{
92+
"X-RateLimit-Remaining": "0",
93+
"X-RateLimit-Reset": strconv.FormatInt(time.Now().Add(-3*time.Minute).Unix(), 10),
94+
})
95+
msg := apiError("https://api.github.com/x", r).Error()
96+
if strings.Contains(msg, "resets in -") || strings.Contains(msg, "resets in 0s") {
97+
t.Errorf("a past reset stamp must be omitted, got: %s", msg)
98+
}
99+
if !strings.Contains(msg, "rate limit exceeded") {
100+
t.Errorf("still a rate limit, got: %s", msg)
101+
}
102+
}
103+
104+
func TestMalformedResetHeaderIsIgnored(t *testing.T) {
105+
r := resp(http.StatusForbidden, map[string]string{
106+
"X-RateLimit-Remaining": "0",
107+
"X-RateLimit-Reset": "not-a-number",
108+
})
109+
msg := apiError("https://api.github.com/x", r).Error()
110+
if strings.Contains(msg, "resets in") {
111+
t.Errorf("an unparseable reset header must be dropped, got: %s", msg)
112+
}
113+
}

‎internal/selfupdate/selfupdate.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,7 @@ func githubGET(ctx context.Context, url string) ([]byte, error) {
167167
defer resp.Body.Close()
168168
body, _ := io.ReadAll(io.LimitReader(resp.Body, 4<<20))
169169
if resp.StatusCode != http.StatusOK {
170-
return nil, fmt.Errorf("GitHub API %s returned %s (set GITHUB_TOKEN if the repo is private)", url, resp.Status)
170+
return nil, apiError(url, resp)
171171
}
172172
return body, nil
173173
}

‎internal/selfupdate/update.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -249,7 +249,7 @@ func downloadAsset(ctx context.Context, url string) ([]byte, error) {
249249
}
250250
defer resp.Body.Close()
251251
if resp.StatusCode != http.StatusOK {
252-
return nil, fmt.Errorf("%s returned %s", url, resp.Status)
252+
return nil, apiError(url, resp)
253253
}
254254
return io.ReadAll(io.LimitReader(resp.Body, 200<<20))
255255
}

0 commit comments

Comments
 (0)