Merge main into reorg - resolved conflicts and moved security files to cmd/goose

Author: tstromberg

.claude/CLAUDE.md

@@ -0,0 +1,89 @@
+# CLAUDE.md
+
+This file provides guidance to Claude Code (claude.ai/code) when working with code in this repository.
+
+## Project Overview
+
+Ready to Review is a macOS/Linux/Windows menubar application that helps developers track GitHub pull requests. It shows a count of incoming/outgoing PRs and highlights when you're blocking someone's review. The app integrates with the Turn API to provide intelligent PR metadata about who's actually blocking progress.
+
+## Key Commands
+
+### Building and Running
+```bash
+make run          # Build and run (on macOS: installs to /Applications and launches)
+make build        # Build for current platform
+make app-bundle   # Create macOS .app bundle
+make install      # Install to system (macOS: /Applications, Linux: /usr/local/bin, Windows: %LOCALAPPDATA%)
+```
+
+### Development
+```bash
+make lint         # Run all linters (golangci-lint with strict config + yamllint)
+make fix          # Auto-fix linting issues where possible
+make deps         # Download and tidy Go dependencies
+make clean        # Remove build artifacts
+```
+
+## Architecture Overview
+
+### Core Components
+
+1. **Main Application Flow** (`main.go`)
+   - Single `context.Background()` created in main, passed through all functions
+   - App struct holds GitHub/Turn clients, PR data, and UI state
+   - Update loop runs every 2 minutes to refresh PR data
+   - Menu updates only rebuild when PR data actually changes (hash-based optimization)
+
+2. **GitHub Integration** (`github.go`)
+   - Uses GitHub CLI token (`gh auth token`) for authentication
+   - Fetches PRs with a single optimized query: `is:open is:pr involves:USER archived:false`
+   - No pagination needed (uses 100 per page limit)
+
+3. **Turn API Integration** (`cache.go`)
+   - Provides intelligent PR metadata (who's blocking, PR size, tags)
+   - Implements 2-hour TTL cache with SHA256-based cache keys
+   - Cache cleanup runs daily, removes files older than 5 days
+   - Turn API calls are made for each PR to determine blocking status
+
+4. **UI Management** (`ui.go`)
+   - System tray integration via energye/systray
+   - Menu structure: Incoming PRs → Outgoing PRs → Settings → About
+   - Click handlers open PRs in browser with URL validation
+   - "Hide stale PRs" option filters PRs older than 90 days
+
+5. **Platform-Specific Features**
+   - `loginitem_darwin.go`: macOS "Start at Login" functionality via AppleScript
+   - `loginitem_other.go`: Stub for non-macOS platforms
+
+### Key Design Decisions
+
+- **No Context in Structs**: Context is always passed as a parameter, never stored
+- **Graceful Degradation**: Turn API failures don't break the app, PRs still display
+- **Security**: Only HTTPS URLs allowed, whitelist of github.com and dash.ready-to-review.dev
+- **Minimal Dependencies**: Uses standard library where possible
+- **Proper Cancellation**: All goroutines respect context cancellation
+
+### Linting Configuration
+
+The project uses an extremely strict golangci-lint configuration (`.golangci.yml`) that enforces:
+- All available linters except those that conflict with Go best practices
+- No nolint directives without explanations
+- Cognitive complexity limit of 55
+- No magic numbers (must use constants)
+- Proper error handling (no unchecked errors)
+- No naked returns except in very short functions
+- Field alignment optimization for structs
+
+### Special Considerations
+
+1. **Authentication**: Uses GitHub CLI token, never stores it persistently
+2. **Caching**: Turn API responses cached to reduce API calls
+3. **Menu Updates**: Hash-based change detection prevents unnecessary UI updates
+4. **Context Handling**: Single context from main, proper cancellation in all goroutines
+5. **Error Handling**: All errors wrapped with context using `fmt.Errorf` with `%w`
+
+When making changes:
+- Run `make lint` and fix all issues without adding nolint directives
+- Follow the strict Go style guidelines in ~/.claude/CLAUDE.md
+- Keep functions simple and focused
+- Test macOS-specific features carefully (login items, app bundle)

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,34 @@ 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:
+
+For maximum security, use a [fine-grained personal access token](https://github.com/settings/personal-access-tokens/new):
+
+1. Go to GitHub Settings → Developer settings → Personal access tokens → Fine-grained tokens
+2. Create a new token with:
+   - **Expiration**: Set a short expiration (30-90 days recommended)
+   - **Repository access**: Select only the specific repositories you want to monitor
+   - **Permissions**:
+     - Pull requests: Read
+     - Metadata: Read
+3. Copy the token (starts with `github_pat_`)
+
+If you need broader access, you can use a [classic token](https://github.com/settings/tokens):
+- Create with `repo` scope (grants full repository access - use with caution)
+
+#### Using the Token
+
+```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. Fine-grained tokens are strongly recommended for better security.
+
 ## Known Issues
 
 - Blocking logic isn't 100% accurate (we're working on it)
@@ -53,10 +83,12 @@ This tool is part of the [CodeGroove](https://codegroove.dev) developer accelera
 
 ## Privacy
 
-Your GitHub token used to fetch PR metadata but we never store it anywhere. GitHub data is retained strictly for caching purposes with a 20-day maximum TTL.
+- Your GitHub token is used to fetch PR metadata, but is never stored or logged.
+- We won't sell your information or use it for any purpose other than caching.
+- GitHub metadata for open pull requests may be cached for up to 20 days for performance reasons.
 
 ---
 
-Built with ❤️ and mild sleep deprivation by [CodeGroove](https://codegroove.dev/products/)
+Built with ❤️ by [CodeGroove](https://codegroove.dev/products/)
 
 [Contribute](https://github.com/ready-to-review/goose) (PRs welcome, but the goose will judge you)

cmd/goose/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))
@@ -59,7 +64,7 @@ func (app *App) turnData(ctx context.Context, url string, updatedAt time.Time) (
 	var data *turn.CheckResponse
 	err := retry.Do(func() error {
 		// Create timeout context for Turn API call
-		turnCtx, cancel := context.WithTimeout(ctx, 10*time.Second)
+		turnCtx, cancel := context.WithTimeout(ctx, turnAPITimeout)
 		defer cancel()
 
 		var retryErr error
@@ -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)
 			}
 		}
 	}

cmd/goose/github.go

@@ -9,7 +9,6 @@ import (
 	"os"
 	"os/exec"
 	"path/filepath"
-	"regexp"
 	"runtime"
 	"strings"
 	"sync"
@@ -21,9 +20,6 @@ import (
 	"golang.org/x/oauth2"
 )
 
-// githubTokenRegex matches valid GitHub token formats.
-var githubTokenRegex = regexp.MustCompile(`^(ghp_[a-zA-Z0-9]{36}|gho_[a-zA-Z0-9]{36}|github_pat_[a-zA-Z0-9]{22}_[a-zA-Z0-9]{59})$`)
-
 // initClients initializes GitHub and Turn API clients.
 func (app *App) initClients(ctx context.Context) error {
 	token, err := app.token(ctx)
@@ -63,7 +59,6 @@ func (*App) token(ctx context.Context) (string, error) {
 		log.Println("Using GitHub token from GITHUB_TOKEN environment variable")
 		return token, nil
 	}
-
 	// Only check absolute paths for security - never use PATH
 	var trustedPaths []string
 	switch runtime.GOOS {
@@ -103,8 +98,11 @@ func (*App) token(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
@@ -115,25 +113,21 @@ func (*App) token(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")
+	if err := validateGitHubToken(token); err != nil {
+		return "", fmt.Errorf("invalid token from gh CLI: %w", err)
 	}
-	const minTokenLength = 20
-	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
 }
 
@@ -400,7 +394,10 @@ func (app *App) fetchTurnDataSync(ctx context.Context, issues []*github.Issue, u
 	// Use a WaitGroup to track goroutines
 	var wg sync.WaitGroup
 
-	// Process PRs in parallel
+	// Create semaphore to limit concurrent Turn API calls
+	sem := make(chan struct{}, maxConcurrentTurnAPICalls)
+
+	// Process PRs in parallel with concurrency limit
 	for _, issue := range issues {
 		if !issue.IsPullRequest() {
 			continue
@@ -410,6 +407,10 @@ func (app *App) fetchTurnDataSync(ctx context.Context, issues []*github.Issue, u
 		go func(issue *github.Issue) {
 			defer wg.Done()
 
+			// Acquire semaphore
+			sem <- struct{}{}
+			defer func() { <-sem }()
+
 			url := issue.GetHTMLURL()
 			updatedAt := issue.GetUpdatedAt().Time
 
@@ -490,7 +491,10 @@ func (app *App) fetchTurnDataAsync(ctx context.Context, issues []*github.Issue,
 	// Use a WaitGroup to track goroutines
 	var wg sync.WaitGroup
 
-	// Process PRs in parallel
+	// Create semaphore to limit concurrent Turn API calls
+	sem := make(chan struct{}, maxConcurrentTurnAPICalls)
+
+	// Process PRs in parallel with concurrency limit
 	for _, issue := range issues {
 		if !issue.IsPullRequest() {
 			continue
@@ -500,6 +504,10 @@ func (app *App) fetchTurnDataAsync(ctx context.Context, issues []*github.Issue,
 		go func(issue *github.Issue) {
 			defer wg.Done()
 
+			// Acquire semaphore
+			sem <- struct{}{}
+			defer func() { <-sem }()
+
 			url := issue.GetHTMLURL()
 			updatedAt := issue.GetUpdatedAt().Time
 

cmd/goose/main.go

@@ -48,6 +48,19 @@ const (
 	// Retry settings for external API calls - exponential backoff with jitter up to 2 minutes.
 	maxRetryDelay = 2 * time.Minute
 	maxRetries    = 10 // Should reach 2 minutes with exponential backoff
+
+	// Failure thresholds.
+	minorFailureThreshold = 3
+	majorFailureThreshold = 10
+	panicFailureIncrement = 10
+
+	// Notification settings.
+	reminderInterval     = 24 * time.Hour
+	historyRetentionDays = 30
+
+	// Turn API settings.
+	turnAPITimeout            = 10 * time.Second
+	maxConcurrentTurnAPICalls = 10
 )
 
 // PR represents a pull request with metadata.
@@ -131,6 +144,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)
@@ -198,9 +218,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...")
@@ -264,7 +284,7 @@ func (app *App) updateLoop(ctx context.Context) {
 
 			// Update failure count
 			app.mu.Lock()
-			app.consecutiveFailures += 10 // Treat panic as critical failure
+			app.consecutiveFailures += panicFailureIncrement // Treat panic as critical failure
 			app.mu.Unlock()
 
 			// Signal app to quit after panic
@@ -302,10 +322,6 @@ func (app *App) updatePRs(ctx context.Context) {
 		app.mu.Unlock()
 
 		// Progressive degradation based on failure count
-		const (
-			minorFailureThreshold = 3
-			majorFailureThreshold = 10
-		)
 		var title, tooltip string
 		switch {
 		case failureCount == 1:
@@ -470,10 +486,6 @@ func (app *App) updatePRsWithWait(ctx context.Context) {
 		app.mu.Unlock()
 
 		// Progressive degradation based on failure count
-		const (
-			minorFailureThreshold = 3
-			majorFailureThreshold = 10
-		)
 		var title, tooltip string
 		switch {
 		case failureCount == 1:
@@ -658,8 +670,7 @@ func (app *App) checkForNewlyBlockedPRs(ctx context.Context) {
 	now := time.Now()
 	staleThreshold := now.Add(-stalePRThreshold)
 
-	// Reminder interval for re-notifications (24 hours)
-	const reminderInterval = 24 * time.Hour
+	// Use reminder interval constant from package level
 
 	currentBlockedPRs := make(map[string]bool)
 	playedIncomingSound := false