Merge pull request #15 from ready-to-review/token-env

Add support for GITHUB_TOKEN environment variable

Author: tstromberg

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)

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)
 			}
 		}
 	}

github.go

@@ -44,8 +44,19 @@ 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)
+		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
+	}
+
+	// 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 {
@@ -85,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
@@ -97,25 +111,21 @@ 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)
 	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")
+	log.Println("Successfully obtained GitHub token from gh CLI")
 	return token, nil
 }
 
@@ -200,9 +210,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 +246,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 +269,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))

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...")