Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 54 additions & 10 deletions internal/dispatch/gcf/mintsrc/mintcore/github.go.embed
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,30 @@ import (
"time"
)

// GitHubAPIError is returned when the GitHub API responds with a non-success
// status code. It preserves the upstream status code and response body so
// callers can distinguish client errors (4xx) from server errors (5xx) and
// surface GitHub's error message in logs and client responses.
type GitHubAPIError struct {
StatusCode int
Body string
Action string // what we were doing, e.g. "creating installation token"
}

func (e *GitHubAPIError) Error() string {
if e.Body != "" {
return fmt.Sprintf("%s returned status %d: %s", e.Action, e.StatusCode, e.Body)
}
return fmt.Sprintf("%s returned status %d", e.Action, e.StatusCode)
}

// readErrorBody reads up to maxBytes from r and returns the content as a
// trimmed string.
func readErrorBody(r io.Reader, maxBytes int64) string {
body, _ := io.ReadAll(io.LimitReader(r, maxBytes))
return strings.TrimSpace(string(body))
}

// installationResponse is the response from GET /repos/{owner}/{repo}/installation.
type installationResponse struct {
ID int64 `json:"id"`
Expand Down Expand Up @@ -253,8 +277,12 @@ func FindInstallation(ctx context.Context, httpClient HTTPDoer, githubBaseURL, j
defer resp.Body.Close()

if resp.StatusCode != http.StatusOK {
io.Copy(io.Discard, io.LimitReader(resp.Body, 4096))
return 0, fmt.Errorf("getting installation for %s/%s returned status %d", org, repo, resp.StatusCode)
body := readErrorBody(resp.Body, 4096)
return 0, &GitHubAPIError{
StatusCode: resp.StatusCode,
Body: body,
Action: fmt.Sprintf("getting installation for %s/%s", org, repo),
}
}

var inst installationResponse
Expand Down Expand Up @@ -293,8 +321,12 @@ func FindOrgInstallation(ctx context.Context, httpClient HTTPDoer, githubBaseURL
defer resp.Body.Close()

if resp.StatusCode != http.StatusOK {
io.Copy(io.Discard, io.LimitReader(resp.Body, 4096))
return 0, fmt.Errorf("getting org installation for %s returned status %d", org, resp.StatusCode)
body := readErrorBody(resp.Body, 4096)
return 0, &GitHubAPIError{
StatusCode: resp.StatusCode,
Body: body,
Action: fmt.Sprintf("getting org installation for %s", org),
}
}

var inst installationResponse
Expand Down Expand Up @@ -340,8 +372,12 @@ func GetOrgVariable(ctx context.Context, httpClient HTTPDoer, githubBaseURL, ins
return "", false, nil
}
if resp.StatusCode != http.StatusOK {
io.Copy(io.Discard, io.LimitReader(resp.Body, 4096))
return "", false, fmt.Errorf("getting org variable %s returned status %d", name, resp.StatusCode)
body := readErrorBody(resp.Body, 4096)
return "", false, &GitHubAPIError{
StatusCode: resp.StatusCode,
Body: body,
Action: fmt.Sprintf("getting org variable %s", name),
}
}

var varResp orgVariableResponse
Expand Down Expand Up @@ -386,8 +422,12 @@ func createInstallationTokenWithPermissions(ctx context.Context, httpClient HTTP
defer resp.Body.Close()

if resp.StatusCode != http.StatusCreated {
io.Copy(io.Discard, io.LimitReader(resp.Body, 4096))
return "", fmt.Errorf("creating installation token returned status %d", resp.StatusCode)
body := readErrorBody(resp.Body, 4096)
return "", &GitHubAPIError{
StatusCode: resp.StatusCode,
Body: body,
Action: "creating installation token",
}
}

var tokenResp installationTokenResponse
Expand Down Expand Up @@ -453,8 +493,12 @@ func CreateInstallationToken(ctx context.Context, httpClient HTTPDoer, githubBas
defer resp.Body.Close()

if resp.StatusCode != http.StatusCreated {
io.Copy(io.Discard, io.LimitReader(resp.Body, 4096))
return "", "", nil, fmt.Errorf("creating installation token returned status %d", resp.StatusCode)
body := readErrorBody(resp.Body, 4096)
return "", "", nil, &GitHubAPIError{
StatusCode: resp.StatusCode,
Body: body,
Action: "creating installation token",
}
}

var tokenResp installationTokenResponse
Expand Down
20 changes: 16 additions & 4 deletions internal/dispatch/gcf/mintsrc/mintcore/handler.go.embed
Original file line number Diff line number Diff line change
Expand Up @@ -365,12 +365,12 @@ func (h *Handler) mintToken(ctx context.Context, org, role string, repos []strin
installationID, err = FindInstallation(ctx, h.httpClient, h.githubBaseURL, jwt, org, repos[0])
}
if err != nil {
return "", "", nil, &mintError{status: http.StatusBadGateway, msg: err.Error()}
return "", "", nil, &mintError{status: upstreamStatus(err), msg: err.Error()}
}

token, expiresAt, granted, err := CreateInstallationToken(ctx, h.httpClient, h.githubBaseURL, jwt, installationID, role, repos)
if err != nil {
return "", "", nil, &mintError{status: http.StatusBadGateway, msg: err.Error()}
return "", "", nil, &mintError{status: upstreamStatus(err), msg: err.Error()}
}

if granted != nil {
Expand All @@ -384,7 +384,7 @@ func (h *Handler) mintToken(ctx context.Context, org, role string, repos []strin
func (h *Handler) mintTokenCrossOrg(ctx context.Context, claims *Claims, targetOrg, role string, repos []string) (string, string, *GrantedScope, error) {
allowlist, err := h.loadForeignAllowlist(ctx, targetOrg, role)
if err != nil {
return "", "", nil, &mintError{status: http.StatusBadGateway, msg: err.Error()}
return "", "", nil, &mintError{status: upstreamStatus(err), msg: err.Error()}
}
if len(allowlist) == 0 {
return "", "", nil, &mintError{status: http.StatusForbidden, msg: "foreign caller not authorized for target org"}
Expand Down Expand Up @@ -462,7 +462,7 @@ func (h *Handler) fetchForeignAllowlist(ctx context.Context, targetOrg, role str

installationID, err := FindOrgInstallation(ctx, h.httpClient, h.githubBaseURL, jwt, targetOrg)
if err != nil {
return nil, fmt.Errorf("finding org installation on %s: %v", targetOrg, err)
return nil, fmt.Errorf("finding org installation on %s: %w", targetOrg, err)
}

allowlist, err := ReadForeignAllowlist(ctx, h.httpClient, h.githubBaseURL, jwt, installationID, targetOrg, role)
Expand Down Expand Up @@ -537,6 +537,18 @@ func (h *Handler) lookupRoleAppID(role string) (string, error) {
return appID, nil
}

// upstreamStatus inspects err for a GitHubAPIError. If GitHub returned a
// 4xx client error the caller's request is at fault and will never succeed on
// retry, so we forward that status. For 5xx or non-GitHub errors we return 502
// (Bad Gateway) so clients know to retry.
func upstreamStatus(err error) int {
var ghErr *GitHubAPIError
if errors.As(err, &ghErr) && ghErr.StatusCode >= 400 && ghErr.StatusCode < 500 {
return ghErr.StatusCode
}
return http.StatusBadGateway
}

// mintError is an HTTP-aware error carrying a status code for the response.
type mintError struct {
status int
Expand Down
64 changes: 54 additions & 10 deletions internal/mintcore/github.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,30 @@ import (
"time"
)

// GitHubAPIError is returned when the GitHub API responds with a non-success
// status code. It preserves the upstream status code and response body so
// callers can distinguish client errors (4xx) from server errors (5xx) and
// surface GitHub's error message in logs and client responses.
type GitHubAPIError struct {
StatusCode int
Body string
Action string // what we were doing, e.g. "creating installation token"
}

func (e *GitHubAPIError) Error() string {
if e.Body != "" {
return fmt.Sprintf("%s returned status %d: %s", e.Action, e.StatusCode, e.Body)
}
return fmt.Sprintf("%s returned status %d", e.Action, e.StatusCode)
}

// readErrorBody reads up to maxBytes from r and returns the content as a
// trimmed string.
func readErrorBody(r io.Reader, maxBytes int64) string {
body, _ := io.ReadAll(io.LimitReader(r, maxBytes))
return strings.TrimSpace(string(body))
}

// installationResponse is the response from GET /repos/{owner}/{repo}/installation.
type installationResponse struct {
ID int64 `json:"id"`
Expand Down Expand Up @@ -253,8 +277,12 @@ func FindInstallation(ctx context.Context, httpClient HTTPDoer, githubBaseURL, j
defer resp.Body.Close()

if resp.StatusCode != http.StatusOK {
io.Copy(io.Discard, io.LimitReader(resp.Body, 4096))
return 0, fmt.Errorf("getting installation for %s/%s returned status %d", org, repo, resp.StatusCode)
body := readErrorBody(resp.Body, 4096)
return 0, &GitHubAPIError{
StatusCode: resp.StatusCode,
Body: body,
Action: fmt.Sprintf("getting installation for %s/%s", org, repo),
}
}

var inst installationResponse
Expand Down Expand Up @@ -293,8 +321,12 @@ func FindOrgInstallation(ctx context.Context, httpClient HTTPDoer, githubBaseURL
defer resp.Body.Close()

if resp.StatusCode != http.StatusOK {
io.Copy(io.Discard, io.LimitReader(resp.Body, 4096))
return 0, fmt.Errorf("getting org installation for %s returned status %d", org, resp.StatusCode)
body := readErrorBody(resp.Body, 4096)
return 0, &GitHubAPIError{
StatusCode: resp.StatusCode,
Body: body,
Action: fmt.Sprintf("getting org installation for %s", org),
}
}

var inst installationResponse
Expand Down Expand Up @@ -340,8 +372,12 @@ func GetOrgVariable(ctx context.Context, httpClient HTTPDoer, githubBaseURL, ins
return "", false, nil
}
if resp.StatusCode != http.StatusOK {
io.Copy(io.Discard, io.LimitReader(resp.Body, 4096))
return "", false, fmt.Errorf("getting org variable %s returned status %d", name, resp.StatusCode)
body := readErrorBody(resp.Body, 4096)
return "", false, &GitHubAPIError{
StatusCode: resp.StatusCode,
Body: body,
Action: fmt.Sprintf("getting org variable %s", name),
}
}

var varResp orgVariableResponse
Expand Down Expand Up @@ -386,8 +422,12 @@ func createInstallationTokenWithPermissions(ctx context.Context, httpClient HTTP
defer resp.Body.Close()

if resp.StatusCode != http.StatusCreated {
io.Copy(io.Discard, io.LimitReader(resp.Body, 4096))
return "", fmt.Errorf("creating installation token returned status %d", resp.StatusCode)
body := readErrorBody(resp.Body, 4096)
return "", &GitHubAPIError{
StatusCode: resp.StatusCode,
Body: body,
Action: "creating installation token",
}
}

var tokenResp installationTokenResponse
Expand Down Expand Up @@ -453,8 +493,12 @@ func CreateInstallationToken(ctx context.Context, httpClient HTTPDoer, githubBas
defer resp.Body.Close()

if resp.StatusCode != http.StatusCreated {
io.Copy(io.Discard, io.LimitReader(resp.Body, 4096))
return "", "", nil, fmt.Errorf("creating installation token returned status %d", resp.StatusCode)
body := readErrorBody(resp.Body, 4096)
return "", "", nil, &GitHubAPIError{
StatusCode: resp.StatusCode,
Body: body,
Action: "creating installation token",
}
}

var tokenResp installationTokenResponse
Expand Down
65 changes: 65 additions & 0 deletions internal/mintcore/github_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -419,21 +419,86 @@ func TestReadForeignAllowlist_EmptyVariable(t *testing.T) {
func TestFindOrgInstallation_NotFound(t *testing.T) {
mockGH := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusNotFound)
w.Write([]byte(`{"message":"Not Found"}`))
}))
defer mockGH.Close()

_, err := FindOrgInstallation(t.Context(), http.DefaultClient, mockGH.URL, "fake-jwt", "myorg")
require.Error(t, err)
assert.Contains(t, err.Error(), "status 404")

var ghErr *GitHubAPIError
require.ErrorAs(t, err, &ghErr)
assert.Equal(t, 404, ghErr.StatusCode)
assert.Contains(t, ghErr.Body, "Not Found")
}

func TestCreateInstallationToken_NonCreatedStatus_IncludesBody(t *testing.T) {
ghBody := `{"message":"Validation Failed","errors":[{"resource":"Repository","code":"invalid"}]}`
mockGH := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusUnprocessableEntity)
w.Write([]byte(ghBody))
}))
defer mockGH.Close()

_, _, _, err := CreateInstallationToken(t.Context(), http.DefaultClient, mockGH.URL, "fake-jwt", 42, "coder", []string{"stale-repo"})
require.Error(t, err)
assert.Contains(t, err.Error(), "422")
assert.Contains(t, err.Error(), "Validation Failed")

var ghErr *GitHubAPIError
require.ErrorAs(t, err, &ghErr)
assert.Equal(t, http.StatusUnprocessableEntity, ghErr.StatusCode)
assert.Contains(t, ghErr.Body, "Validation Failed")
}

func TestCreateInstallationToken_5xxStatus_IncludesBody(t *testing.T) {
mockGH := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusInternalServerError)
w.Write([]byte(`{"message":"Internal Server Error"}`))
}))
defer mockGH.Close()

_, _, _, err := CreateInstallationToken(t.Context(), http.DefaultClient, mockGH.URL, "fake-jwt", 42, "coder", []string{"repo"})
require.Error(t, err)
assert.Contains(t, err.Error(), "500")
assert.Contains(t, err.Error(), "Internal Server Error")

var ghErr *GitHubAPIError
require.ErrorAs(t, err, &ghErr)
assert.Equal(t, http.StatusInternalServerError, ghErr.StatusCode)
}

func TestFindInstallation_ErrorIncludesBody(t *testing.T) {
mockGH := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusForbidden)
w.Write([]byte(`{"message":"Resource not accessible by integration"}`))
}))
defer mockGH.Close()

_, err := FindInstallation(t.Context(), http.DefaultClient, mockGH.URL, "fake-jwt", "myorg", "my-repo")
require.Error(t, err)
assert.Contains(t, err.Error(), "403")
assert.Contains(t, err.Error(), "Resource not accessible")

var ghErr *GitHubAPIError
require.ErrorAs(t, err, &ghErr)
assert.Equal(t, http.StatusForbidden, ghErr.StatusCode)
}

func TestGetOrgVariable_ErrorStatus(t *testing.T) {
mockGH := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusForbidden)
w.Write([]byte(`{"message":"Resource not accessible"}`))
}))
defer mockGH.Close()

_, _, err := GetOrgVariable(t.Context(), http.DefaultClient, mockGH.URL, "ghs_policy", "pool-org", "VAR")
require.Error(t, err)
assert.Contains(t, err.Error(), "status 403")

var ghErr *GitHubAPIError
require.ErrorAs(t, err, &ghErr)
assert.Equal(t, 403, ghErr.StatusCode)
assert.Contains(t, ghErr.Body, "Resource not accessible")
}
Loading
Loading