Skip to content

bug(binary): empty case http.StatusForbidden treats 403 downloads as success #161

@fentas

Description

@fentas

Summary

In pkg/binary/download.go, the HTTP status switch has an empty case http.StatusForbidden: that silently falls through to no error. Go switch cases do not fall through, so a 403 matches the empty case, returns nil, and the download proceeds to read the body of a forbidden response as if it succeeded — producing a corrupt/garbage binary or a confusing downstream error instead of a clear "forbidden" message.

// pkg/binary/download.go (~line 493)
switch resp.StatusCode {
case http.StatusNotFound:
    // ... returns a useful error
case http.StatusForbidden:        // <-- empty: returns nil, falls through to success path
case http.StatusUnauthorized:
    return fmt.Errorf("Unauthorized")
case http.StatusTooManyRequests:
    return fmt.Errorf("Rate limited")
default:
    return fmt.Errorf("HTTP error %d", resp.StatusCode)
}

Expected

A 403 should return a clear error (e.g. Forbidden / Unauthorized), not be treated as a successful download.

Notes

  • Pre-existing on main; spotted during the review of fix(state): keep relative file paths on save; clearer install errors #159 but left out of scope to keep that PR focused.
  • Likely intent was to group StatusForbidden with StatusUnauthorized (i.e. case http.StatusForbidden, http.StatusUnauthorized:).
  • Worth adding a small test that serves a 403 via httptest and asserts a non-nil error.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions