From f9f349636f8811d11d605638cc4cf34f29a5d767 Mon Sep 17 00:00:00 2001 From: Thomas Stromberg Date: Tue, 12 Aug 2025 15:10:53 -0400 Subject: [PATCH 1/2] Add support for GITHUB_TOKEN environment variable --- README.md | 17 +++++++++++++++++ github.go | 27 +++++++++++++++++++++++---- 2 files changed, 40 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 2ba84da..2cac37b 100644 --- a/README.md +++ b/README.md @@ -24,6 +24,8 @@ You can also visit the web-based equivalent at https://dash.ready-to-review.dev/ ## macOS Quick Start ⚡ (How to Get Honked At) +### Option 1: Using GitHub CLI (Default) + Install dependencies: the [GitHub CLI, aka "gh"](https://cli.github.com/) and [Go](https://go.dev/): ```bash @@ -38,6 +40,21 @@ git clone https://github.com/ready-to-review/goose.git cd goose && make run ``` +### Option 2: Using a GitHub Token (More Control) + +If you want more control over which repositories the goose can access, you can use a GitHub personal access token instead: + +1. Create a [GitHub personal access token](https://github.com/settings/tokens) with `repo` scope +2. Set the `GITHUB_TOKEN` environment variable: + +```bash +export GITHUB_TOKEN=your_token_here +git clone https://github.com/ready-to-review/goose.git +cd goose && make run +``` + +When `GITHUB_TOKEN` is set, the goose will use it directly instead of the GitHub CLI, giving you precise control over repository access. + ## Known Issues - Blocking logic isn't 100% accurate (we're working on it) diff --git a/github.go b/github.go index e3fb1ab..6c05433 100644 --- a/github.go +++ b/github.go @@ -44,8 +44,20 @@ func (app *App) initClients(ctx context.Context) error { return nil } -// githubToken retrieves the GitHub token using gh CLI. +// githubToken retrieves the GitHub token from GITHUB_TOKEN env var or gh CLI. func (*App) githubToken(ctx context.Context) (string, error) { + // First check for GITHUB_TOKEN environment variable + if token := os.Getenv("GITHUB_TOKEN"); token != "" { + token = strings.TrimSpace(token) + const minTokenLength = 20 + if len(token) < minTokenLength { + return "", fmt.Errorf("invalid GITHUB_TOKEN length: %d", len(token)) + } + log.Println("Using GitHub token from GITHUB_TOKEN environment variable") + return token, nil + } + + // Fall back to gh CLI if GITHUB_TOKEN is not set // Only check absolute paths for security - never use PATH var trustedPaths []string switch runtime.GOOS { @@ -97,7 +109,7 @@ func (*App) githubToken(ctx context.Context) (string, error) { } if ghPath == "" { - return "", errors.New("gh cli not found in trusted locations") + return "", errors.New("gh cli not found in trusted locations and GITHUB_TOKEN not set") } log.Printf("Executing command: %s auth token", ghPath) @@ -115,7 +127,7 @@ func (*App) githubToken(ctx context.Context) (string, error) { if len(token) < minTokenLength { return "", fmt.Errorf("invalid github token length: %d", len(token)) } - log.Println("Successfully obtained GitHub token") + log.Println("Successfully obtained GitHub token from gh CLI") return token, nil } @@ -200,9 +212,9 @@ func (app *App) fetchPRsInternal(ctx context.Context, waitForTurn bool) (incomin // Run both queries in parallel type queryResult struct { + err error query string issues []*github.Issue - err error } queryResults := make(chan queryResult, 2) @@ -236,11 +248,13 @@ func (app *App) fetchPRsInternal(ctx context.Context, waitForTurn bool) (incomin // Collect results from both queries var allIssues []*github.Issue seenURLs := make(map[string]bool) + var queryErrors []error for range 2 { result := <-queryResults if result.err != nil { log.Printf("[GITHUB] Query failed: %s - %v", result.query, result.err) + queryErrors = append(queryErrors, result.err) // Continue processing other query results even if one fails continue } @@ -257,6 +271,11 @@ func (app *App) fetchPRsInternal(ctx context.Context, waitForTurn bool) (incomin } log.Printf("[GITHUB] Both searches completed in %v, found %d unique PRs", time.Since(searchStart), len(allIssues)) + // If both queries failed, return an error + if len(queryErrors) == 2 { + return nil, nil, fmt.Errorf("all GitHub queries failed: %v", queryErrors) + } + // Limit PRs for performance if len(allIssues) > maxPRsToProcess { log.Printf("Limiting to %d PRs for performance (total: %d)", maxPRsToProcess, len(allIssues)) From d0cd4926c10948c0145a6030fbd36777af20b4f2 Mon Sep 17 00:00:00 2001 From: Thomas Stromberg Date: Tue, 12 Aug 2025 15:23:50 -0400 Subject: [PATCH 2/2] Apply more security extremism --- cache.go | 9 +++++++-- github.go | 22 ++++++++++------------ main.go | 11 +++++++++-- 3 files changed, 26 insertions(+), 16 deletions(-) diff --git a/cache.go b/cache.go index b452286..5034242 100644 --- a/cache.go +++ b/cache.go @@ -25,6 +25,11 @@ type cacheEntry struct { // turnData fetches Turn API data with caching. func (app *App) turnData(ctx context.Context, url string, updatedAt time.Time) (*turn.CheckResponse, bool, error) { + // Validate URL before processing + if err := validateURL(url); err != nil { + return nil, false, fmt.Errorf("invalid URL: %w", err) + } + // Create cache key from URL and updated timestamp key := fmt.Sprintf("%s-%s", url, updatedAt.Format(time.RFC3339)) hash := sha256.Sum256([]byte(key)) @@ -93,11 +98,11 @@ func (app *App) turnData(ctx context.Context, url string, updatedAt time.Time) ( if cacheData, marshalErr := json.Marshal(entry); marshalErr != nil { log.Printf("Failed to marshal cache data for %s: %v", url, marshalErr) } else { - // Ensure cache directory exists + // Ensure cache directory exists with secure permissions if dirErr := os.MkdirAll(filepath.Dir(cacheFile), 0o700); dirErr != nil { log.Printf("Failed to create cache directory: %v", dirErr) } else if writeErr := os.WriteFile(cacheFile, cacheData, 0o600); writeErr != nil { - log.Printf("Failed to write cache file for %s: %v", url, writeErr) + log.Printf("Failed to write cache file: %v", writeErr) } } } diff --git a/github.go b/github.go index 6c05433..7c883a1 100644 --- a/github.go +++ b/github.go @@ -49,9 +49,8 @@ func (*App) githubToken(ctx context.Context) (string, error) { // First check for GITHUB_TOKEN environment variable if token := os.Getenv("GITHUB_TOKEN"); token != "" { token = strings.TrimSpace(token) - const minTokenLength = 20 - if len(token) < minTokenLength { - return "", fmt.Errorf("invalid GITHUB_TOKEN length: %d", len(token)) + if err := validateGitHubToken(token); err != nil { + return "", fmt.Errorf("invalid GITHUB_TOKEN: %w", err) } log.Println("Using GitHub token from GITHUB_TOKEN environment variable") return token, nil @@ -97,8 +96,11 @@ func (*App) githubToken(ctx context.Context) (string, error) { const executableMask = 0o111 if info.Mode().IsRegular() && info.Mode()&executableMask != 0 { // Verify it's actually the gh binary by running version command - cmd := exec.Command(path, "version") //nolint:noctx // Quick version check doesn't need context + // Use timeout to prevent hanging + versionCtx, cancel := context.WithTimeout(ctx, 2*time.Second) + cmd := exec.CommandContext(versionCtx, path, "version") output, err := cmd.Output() + cancel() // Call cancel immediately after command execution if err == nil && strings.Contains(string(output), "gh version") { log.Printf("Found and verified gh at: %s", path) ghPath = path @@ -116,16 +118,12 @@ func (*App) githubToken(ctx context.Context) (string, error) { cmd := exec.CommandContext(ctx, ghPath, "auth", "token") output, err := cmd.CombinedOutput() if err != nil { - log.Printf("gh command failed with output: %s", string(output)) - return "", fmt.Errorf("exec 'gh auth token': %w (output: %s)", err, string(output)) + log.Printf("gh command failed: %v", err) + return "", fmt.Errorf("exec 'gh auth token': %w", err) } token := strings.TrimSpace(string(output)) - if token == "" { - return "", errors.New("empty github token") - } - const minTokenLength = 20 - if len(token) < minTokenLength { - return "", fmt.Errorf("invalid github token length: %d", len(token)) + if err := validateGitHubToken(token); err != nil { + return "", fmt.Errorf("invalid token from gh CLI: %w", err) } log.Println("Successfully obtained GitHub token from gh CLI") return token, nil diff --git a/main.go b/main.go index 2453859..a94faf2 100644 --- a/main.go +++ b/main.go @@ -130,6 +130,13 @@ func main() { flag.DurationVar(&updateInterval, "interval", defaultUpdateInterval, "Update interval (e.g. 30s, 1m, 5m)") flag.Parse() + // Validate target user if provided + if targetUser != "" { + if err := validateGitHubUsername(targetUser); err != nil { + log.Fatalf("Invalid target user: %v", err) + } + } + // Validate update interval if updateInterval < minUpdateInterval { log.Printf("Update interval %v too short, using minimum of %v", updateInterval, minUpdateInterval) @@ -197,9 +204,9 @@ func main() { } app.currentUser = user - // Log if we're using a different target user + // Log if we're using a different target user (sanitized) if app.targetUser != "" && app.targetUser != user.GetLogin() { - log.Printf("Querying PRs for user '%s' instead of authenticated user '%s'", app.targetUser, user.GetLogin()) + log.Printf("Querying PRs for user '%s' instead of authenticated user", sanitizeForLog(app.targetUser)) } log.Println("Starting systray...")