Skip to content
Open
Show file tree
Hide file tree
Changes from 4 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
2 changes: 1 addition & 1 deletion docs/prd/auth-context-multi-identity.md
Original file line number Diff line number Diff line change
Expand Up @@ -1130,7 +1130,7 @@ func processTagStore(

- [Original Issue Report](#problem-statement)
- [Atmos Auth PRD](../../pkg/auth/docs/PRD/PRD-Atmos-Auth.md)
- [AWS SDK Go v2 Configuration](https://aws.github.io/aws-sdk-go-v2/docs/configuring-sdk/)
- [AWS SDK Go v2 Configuration](https://pkg.go.dev/github.com/aws/aws-sdk-go-v2)
- [Error Handling Strategy](error-handling-strategy.md)

## Appendix: Call Chain Diagram
Expand Down
8 changes: 3 additions & 5 deletions pkg/github/artifacts.go
Original file line number Diff line number Diff line change
Expand Up @@ -137,8 +137,7 @@ func defaultArtifactFetcherWithToken(ctx context.Context, token string) *Artifac
// GetPRArtifactInfo retrieves build artifact information for a PR.
// This finds the latest successful workflow run for the PR and locates
// the artifact matching the current platform.
//
// Requires authentication - use GetGitHubTokenOrError() first.
// Works without authentication for public repositories (subject to rate limits).
func GetPRArtifactInfo(ctx context.Context, owner, repo string, prNumber int) (*PRArtifactInfo, error) {
defer perf.Track(nil, "github.GetPRArtifactInfo")()

Expand Down Expand Up @@ -371,7 +370,7 @@ func SupportedPRPlatforms() []string {

// GetPRHeadSHA retrieves the current head commit SHA for a pull request.
// This is used for cache validation to check if the PR has new commits.
// The token parameter is required for API authentication.
// The token parameter is used for API authentication if available; empty string for unauthenticated access.
func GetPRHeadSHA(ctx context.Context, owner, repo string, prNumber int, token string) (string, error) {
defer perf.Track(nil, "github.GetPRHeadSHA")()

Expand All @@ -389,8 +388,7 @@ func (f *ArtifactFetcher) GetPRHeadSHA(ctx context.Context, owner, repo string,
// GetSHAArtifactInfo retrieves build artifact information for a commit SHA.
// This finds the latest successful workflow run for the SHA and locates
// the artifact matching the current platform.
//
// Requires authentication - use GetGitHubTokenOrError() first.
// Works without authentication for public repositories (subject to rate limits).
func GetSHAArtifactInfo(ctx context.Context, owner, repo, sha string) (*SHAArtifactInfo, error) {
defer perf.Track(nil, "github.GetSHAArtifactInfo")()

Expand Down
66 changes: 51 additions & 15 deletions pkg/toolchain/pr_artifact.go
Original file line number Diff line number Diff line change
Expand Up @@ -136,11 +136,8 @@ func CheckPRCacheAndUpdate(ctx context.Context, prNumber int, showProgress bool)
return true, nil
}

// Check for GitHub token.
token, err := github.GetGitHubTokenOrError()
if err != nil {
return false, buildTokenRequiredError()
}
// Get GitHub token if available (not required for public repos).
token := github.GetGitHubToken()

// Get current PR head SHA.
currentSHA, err := github.GetPRHeadSHA(ctx, atmosOwner, atmosRepo, prNumber, token)
Expand Down Expand Up @@ -230,11 +227,8 @@ func InstallFromPR(prNumber int, showProgress bool) (string, error) {

ctx := context.Background()

// Check for GitHub token (required for artifact downloads).
token, err := github.GetGitHubTokenOrError()
if err != nil {
return "", buildTokenRequiredError()
}
// Get GitHub token if available (not required for public repos).
token := github.GetGitHubToken()

// Show progress if requested.
if showProgress {
Expand Down Expand Up @@ -304,7 +298,10 @@ func downloadPRArtifact(ctx context.Context, token string, info *github.PRArtifa
return "", fmt.Errorf("%w: failed to create request: %w", ErrPRArtifactDownloadFailed, err)
}

req.Header.Set("Authorization", "Bearer "+token)
// Only set Authorization header when a token is available.
if token != "" {
req.Header.Set("Authorization", "Bearer "+token)
}
req.Header.Set("Accept", "application/vnd.github+json")

client := &http.Client{
Expand All @@ -328,7 +325,7 @@ func downloadPRArtifact(ctx context.Context, token string, info *github.PRArtifa

if resp.StatusCode != http.StatusOK {
os.Remove(tempPath)
return "", fmt.Errorf("%w: HTTP %d", ErrPRArtifactDownloadFailed, resp.StatusCode)
return "", buildDownloadHTTPError(resp, token)
}

// Copy response to temp file.
Expand Down Expand Up @@ -595,15 +592,54 @@ func listFiles(dir string) ([]string, error) {
return files, err
}

// buildDownloadHTTPError creates a descriptive error for artifact download HTTP failures.
// It distinguishes between auth errors, rate limits, and other failures.
func buildDownloadHTTPError(resp *http.Response, token string) error {
switch resp.StatusCode {
case http.StatusUnauthorized, http.StatusForbidden:
if token == "" {
return buildTokenRequiredError()
}
// Token was provided but rejected.
return errUtils.Build(errUtils.ErrAuthenticationFailed).
WithExplanation("GitHub rejected the provided authentication token").
WithHint("Your token may be invalid or expired").
WithHint("Verify your token: `gh auth status`").
WithHint("Try re-authenticating: `gh auth login`").
WithExitCode(1).
Err()

case http.StatusTooManyRequests:
b := errUtils.Build(errUtils.ErrGitHubRateLimitExceeded).
WithExplanation("GitHub API rate limit exceeded while downloading artifact")
if token == "" {
b = b.WithHint("Unauthenticated requests are limited to 60/hour").
WithHint("Authenticate to increase limit to 5,000/hour: `gh auth login`").
WithHint("Or set `GITHUB_TOKEN` or `ATMOS_GITHUB_TOKEN` environment variable")
} else {
b = b.WithHint("Authenticated requests are limited to 5,000/hour").
WithHint("Wait a few minutes and try again")
}
// Try to extract reset time from response headers.
if resetHeader := resp.Header.Get("X-RateLimit-Reset"); resetHeader != "" {
b = b.WithHintf("Rate limit info: X-RateLimit-Reset=%s", resetHeader)
}
return b.WithExitCode(1).Err()

default:
return fmt.Errorf("%w: HTTP %d", ErrPRArtifactDownloadFailed, resp.StatusCode)
}
}

// buildTokenRequiredError creates a user-friendly error when GitHub token is missing.
func buildTokenRequiredError() error {
b := errUtils.Build(errUtils.ErrAuthenticationFailed).
WithExplanation("GitHub token required to download PR artifacts").
WithHint("Authenticate with GitHub CLI: gh auth login")
WithExplanation("GitHub requires authentication to download this artifact").
WithHint("Authenticate with GitHub CLI: `gh auth login`")

// Show brew install hint on macOS or when brew is available.
if runtime.GOOS == "darwin" || isBrewAvailable() {
b = b.WithHint("Install GitHub CLI: brew install gh")
b = b.WithHint("Install GitHub CLI: `brew install gh`")
}

return b.
Expand Down
179 changes: 179 additions & 0 deletions pkg/toolchain/pr_artifact_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import (
"encoding/json"
"errors"
"fmt"
"net/http"
"net/http/httptest"
"os"
"path/filepath"
"runtime"
Expand Down Expand Up @@ -499,6 +501,183 @@ func TestBuildTokenRequiredError_ContainsHints(t *testing.T) {
assert.ErrorIs(t, err, errUtils.ErrAuthenticationFailed)
}

func TestBuildDownloadHTTPError(t *testing.T) {
tests := []struct {
name string
statusCode int
token string
headers map[string]string
expectedErr error
}{
{
name: "401 without token returns auth required",
statusCode: 401,
token: "",
expectedErr: errUtils.ErrAuthenticationFailed,
},
{
name: "403 without token returns auth required",
statusCode: 403,
token: "",
expectedErr: errUtils.ErrAuthenticationFailed,
},
{
name: "401 with token returns token rejected",
statusCode: 401,
token: "ghp_fake_token",
expectedErr: errUtils.ErrAuthenticationFailed,
},
{
name: "403 with token returns token rejected",
statusCode: 403,
token: "ghp_fake_token",
expectedErr: errUtils.ErrAuthenticationFailed,
},
{
name: "429 without token returns rate limit error",
statusCode: 429,
token: "",
expectedErr: errUtils.ErrGitHubRateLimitExceeded,
},
{
name: "429 with token returns rate limit error",
statusCode: 429,
token: "ghp_fake_token",
expectedErr: errUtils.ErrGitHubRateLimitExceeded,
},
{
name: "429 with reset header returns rate limit error",
statusCode: 429,
token: "",
headers: map[string]string{
"X-RateLimit-Reset": "1710612345",
},
expectedErr: errUtils.ErrGitHubRateLimitExceeded,
},
{
name: "500 returns generic download error",
statusCode: 500,
token: "",
expectedErr: ErrPRArtifactDownloadFailed,
},
{
name: "502 returns generic download error",
statusCode: 502,
token: "ghp_fake_token",
expectedErr: ErrPRArtifactDownloadFailed,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
resp := &http.Response{
StatusCode: tt.statusCode,
Header: http.Header{},
}
for k, v := range tt.headers {
resp.Header.Set(k, v)
}

err := buildDownloadHTTPError(resp, tt.token)
assert.Error(t, err)
assert.ErrorIs(t, err, tt.expectedErr)
})
}
}

func TestDownloadPRArtifact_Success(t *testing.T) {
zipContent := []byte("PK\x03\x04fake-zip-content")
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusOK)
w.Write(zipContent)
}))
defer server.Close()

info := &github.PRArtifactInfo{
DownloadURL: server.URL + "/artifact.zip",
}

path, err := downloadPRArtifact(context.Background(), "test-token", info)
require.NoError(t, err)
defer os.Remove(path)

data, err := os.ReadFile(path)
require.NoError(t, err)
assert.Equal(t, zipContent, data)
}

func TestDownloadPRArtifact_SuccessWithoutToken(t *testing.T) {
var receivedAuth string
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
receivedAuth = r.Header.Get("Authorization")
w.WriteHeader(http.StatusOK)
w.Write([]byte("content"))
}))
defer server.Close()

info := &github.PRArtifactInfo{
DownloadURL: server.URL + "/artifact.zip",
}

path, err := downloadPRArtifact(context.Background(), "", info)
require.NoError(t, err)
defer os.Remove(path)

// Verify no Authorization header was sent.
assert.Empty(t, receivedAuth, "should not send Authorization header without token")
}

func TestDownloadPRArtifact_WithToken(t *testing.T) {
var receivedAuth string
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
receivedAuth = r.Header.Get("Authorization")
w.WriteHeader(http.StatusOK)
w.Write([]byte("content"))
}))
defer server.Close()

info := &github.PRArtifactInfo{
DownloadURL: server.URL + "/artifact.zip",
}

path, err := downloadPRArtifact(context.Background(), "ghp_test123", info)
require.NoError(t, err)
defer os.Remove(path)

assert.Equal(t, "Bearer ghp_test123", receivedAuth)
}

func TestDownloadPRArtifact_AuthError(t *testing.T) {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusForbidden)
}))
defer server.Close()

info := &github.PRArtifactInfo{
DownloadURL: server.URL + "/artifact.zip",
}

_, err := downloadPRArtifact(context.Background(), "", info)
assert.Error(t, err)
assert.ErrorIs(t, err, errUtils.ErrAuthenticationFailed)
}

func TestDownloadPRArtifact_RateLimit(t *testing.T) {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.Header().Set("X-RateLimit-Reset", "1710612345")
w.WriteHeader(http.StatusTooManyRequests)
}))
defer server.Close()

info := &github.PRArtifactInfo{
DownloadURL: server.URL + "/artifact.zip",
}

_, err := downloadPRArtifact(context.Background(), "", info)
assert.Error(t, err)
assert.ErrorIs(t, err, errUtils.ErrGitHubRateLimitExceeded)
}

// Note: Full integration tests for InstallFromPR require:
// - A valid GitHub token
// - Network access
Expand Down
7 changes: 2 additions & 5 deletions pkg/toolchain/sha_artifact.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,11 +97,8 @@ func InstallFromSHA(sha string, showProgress bool) (string, error) {

ctx := context.Background()

// Check for GitHub token (required for artifact downloads).
token, err := github.GetGitHubTokenOrError()
if err != nil {
return "", buildTokenRequiredError()
}
// Get GitHub token if available (not required for public repos).
token := github.GetGitHubToken()

shortSHA := sha
if len(shortSHA) > shortSHALength {
Expand Down
Loading