Skip to content

RateLimit restores quota after a request may have reached GitHub #1349

Description

@gemshrine

Symptom and impact

Fbe::Middleware::RateLimit can report more remaining API quota than GitHub actually has after a request reaches GitHub but its response is lost. Callers such as Fbe::Octo#off_quota? then treat the cached count as authoritative and may start work that is already over quota.

Scenario

  1. The middleware has a known positive @remaining or @searchleft count.
  2. A request is sent to GitHub and the server processes it, consuming one request from that resource.
  3. The client gets a timeout or connection error before receiving the response.
  4. The call rescue invokes untrack_request(took), which restores the decremented local count even though the middleware cannot know whether GitHub processed the request.
  5. A later /rate_limit call can be served from the cached response with this inflated count; Fbe::Octo#off_quota? reads the middleware count without refreshing it.

The same drift can accumulate when several requests time out after reaching the API.

Actual result

The middleware treats every StandardError as proof that the request did not consume quota and restores the counter in untrack_request. A transport failure does not establish that: the server may have completed the request before the response was lost.

Expected result

When delivery is ambiguous, the middleware should not present the restored count as known. It should invalidate or refresh the affected resource count, or otherwise distinguish failures known to occur before the request could reach GitHub from failures that may have happened after processing.

Technical evidence

In lib/fbe/middleware/rate_limit.rb, call rescues StandardError and calls untrack_request(took). For :core and :search, untrack_request increments @remaining or @searchleft. The cached /rate_limit response is then patched with these local values by patched_body. lib/fbe/octo.rb uses @limits[:rate_limit].remaining(resource) in off_quota?, so the inflated count can affect the quota guard.

This is a control-flow finding from the current middleware implementation; no runtime test was run for this report.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions