Skip to content
Merged
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
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ Lives in your menubar like a tiny waterfowl of productivity shame, watching your
- **✈️ Jet sounds** when your own PR is ready for the next stage
- **🧠 Smart turn-based assignment** - knows who is blocking a PR, knows when tests are failing, etc.
- **⭐ Auto-start** on login (macOS)
- **🔔 Auto-open** incoming PRs in your browser (off by default, rate-limited)

You can also visit the web-based equivalent at https://dash.ready-to-review.dev/

Expand Down
119 changes: 119 additions & 0 deletions cmd/goose/browser_rate_limiter.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,119 @@
// Package main implements browser rate limiting for PR auto-open feature.
package main

import (
"log"
"sync"
"time"
)

// BrowserRateLimiter manages rate limiting for automatically opening browser windows.
type BrowserRateLimiter struct {
openedPRs map[string]bool
openedLastMinute []time.Time
openedToday []time.Time
startupDelay time.Duration
maxPerMinute int
maxPerDay int
mu sync.Mutex
}

// NewBrowserRateLimiter creates a new browser rate limiter.
func NewBrowserRateLimiter(startupDelay time.Duration, maxPerMinute, maxPerDay int) *BrowserRateLimiter {
log.Printf("[BROWSER] Initializing rate limiter: startup_delay=%v, max_per_minute=%d, max_per_day=%d",
startupDelay, maxPerMinute, maxPerDay)
return &BrowserRateLimiter{
openedLastMinute: make([]time.Time, 0),
openedToday: make([]time.Time, 0),
startupDelay: startupDelay,
maxPerMinute: maxPerMinute,
maxPerDay: maxPerDay,
openedPRs: make(map[string]bool),
}
}

// CanOpen checks if we can open a browser window according to rate limits.
func (b *BrowserRateLimiter) CanOpen(startTime time.Time, prURL string) bool {
b.mu.Lock()
defer b.mu.Unlock()

// Check if we've already opened this PR
if b.openedPRs[prURL] {
log.Printf("[BROWSER] Skipping auto-open: PR already opened - %s", prURL)
return false
}

// Check startup delay
if time.Since(startTime) < b.startupDelay {
log.Printf("[BROWSER] Skipping auto-open: within startup delay period (%v remaining)",
b.startupDelay-time.Since(startTime))
return false
}

now := time.Now()

// Clean old entries
b.cleanOldEntries(now)

// Check per-minute limit
if len(b.openedLastMinute) >= b.maxPerMinute {
log.Printf("[BROWSER] Rate limit: already opened %d PRs in the last minute (max: %d)",
len(b.openedLastMinute), b.maxPerMinute)
return false
}

// Check per-day limit
if len(b.openedToday) >= b.maxPerDay {
log.Printf("[BROWSER] Rate limit: already opened %d PRs today (max: %d)",
len(b.openedToday), b.maxPerDay)
return false
}

return true
}

// RecordOpen records that a browser window was opened.
func (b *BrowserRateLimiter) RecordOpen(prURL string) {
b.mu.Lock()
defer b.mu.Unlock()

now := time.Now()
b.openedLastMinute = append(b.openedLastMinute, now)
b.openedToday = append(b.openedToday, now)
b.openedPRs[prURL] = true

log.Printf("[BROWSER] Recorded browser open for %s (minute: %d/%d, today: %d/%d)",
prURL, len(b.openedLastMinute), b.maxPerMinute, len(b.openedToday), b.maxPerDay)
}

// cleanOldEntries removes entries outside the time windows.
func (b *BrowserRateLimiter) cleanOldEntries(now time.Time) {
// Clean entries older than 1 minute
oneMinuteAgo := now.Add(-1 * time.Minute)
newLastMinute := make([]time.Time, 0, len(b.openedLastMinute))
for _, t := range b.openedLastMinute {
if t.After(oneMinuteAgo) {
newLastMinute = append(newLastMinute, t)
}
}
b.openedLastMinute = newLastMinute

// Clean entries older than 24 hours (1 day)
oneDayAgo := now.Add(-24 * time.Hour)
newToday := make([]time.Time, 0, len(b.openedToday))
for _, t := range b.openedToday {
if t.After(oneDayAgo) {
newToday = append(newToday, t)
}
}
b.openedToday = newToday
}

// Reset clears the opened PRs tracking - useful when toggling the feature.
func (b *BrowserRateLimiter) Reset() {
b.mu.Lock()
defer b.mu.Unlock()
previousCount := len(b.openedPRs)
b.openedPRs = make(map[string]bool)
log.Printf("[BROWSER] Rate limiter reset: cleared %d tracked PRs", previousCount)
}
55 changes: 53 additions & 2 deletions cmd/goose/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
panicFailureIncrement = 10
turnAPITimeout = 10 * time.Second
maxConcurrentTurnAPICalls = 10
defaultMaxBrowserOpensDay = 20
)

// PR represents a pull request with metadata.
Expand All @@ -69,7 +70,7 @@
}

// App holds the application state.
type App struct {

Check failure on line 73 in cmd/goose/main.go

View workflow job for this annotation

GitHub Actions / golangci-lint

fieldalignment: struct with 280 pointer bytes could be 216 (govet)
lastSuccessfulFetch time.Time
client *github.Client
turnClient *turn.Client
Expand All @@ -90,8 +91,10 @@
loadingTurnData bool
menuInitialized bool
initialLoadComplete bool
enableReminders bool
enableAudioCues bool
enableAutoBrowser bool
browserRateLimiter *BrowserRateLimiter
startTime time.Time
}

func loadCurrentUser(ctx context.Context, app *App) {
Expand Down Expand Up @@ -145,9 +148,15 @@
var targetUser string
var noCache bool
var updateInterval time.Duration
var browserOpenDelay time.Duration
var maxBrowserOpensMinute int
var maxBrowserOpensDay int
flag.StringVar(&targetUser, "user", "", "GitHub user to query PRs for (defaults to authenticated user)")
flag.BoolVar(&noCache, "no-cache", false, "Bypass cache for debugging")
flag.DurationVar(&updateInterval, "interval", defaultUpdateInterval, "Update interval (e.g. 30s, 1m, 5m)")
flag.DurationVar(&browserOpenDelay, "browser-delay", 1*time.Minute, "Minimum delay before opening PRs in browser after startup")
flag.IntVar(&maxBrowserOpensMinute, "browser-max-per-minute", 2, "Maximum browser windows to open per minute")
flag.IntVar(&maxBrowserOpensDay, "browser-max-per-day", defaultMaxBrowserOpensDay, "Maximum browser windows to open per day")
flag.Parse()

// Validate target user if provided
Expand All @@ -163,9 +172,25 @@
updateInterval = minUpdateInterval
}

// Validate browser rate limit parameters
if maxBrowserOpensMinute < 0 {
log.Printf("Invalid browser-max-per-minute %d, using default of 2", maxBrowserOpensMinute)
maxBrowserOpensMinute = 2
}
if maxBrowserOpensDay < 0 {
log.Printf("Invalid browser-max-per-day %d, using default of %d", maxBrowserOpensDay, defaultMaxBrowserOpensDay)
maxBrowserOpensDay = defaultMaxBrowserOpensDay
}
if browserOpenDelay < 0 {
log.Printf("Invalid browser-delay %v, using default of 1 minute", browserOpenDelay)
browserOpenDelay = 1 * time.Minute
}

log.SetFlags(log.LstdFlags | log.Lshortfile)
log.Printf("Starting GitHub PR Monitor (version=%s, commit=%s, date=%s)", version, commit, date)
log.Printf("Configuration: update_interval=%v, max_retries=%d, max_delay=%v", updateInterval, maxRetries, maxRetryDelay)
log.Printf("Browser auto-open: startup_delay=%v, max_per_minute=%d, max_per_day=%d",
browserOpenDelay, maxBrowserOpensMinute, maxBrowserOpensDay)

ctx := context.Background()

Expand All @@ -187,8 +212,10 @@
noCache: noCache,
updateInterval: updateInterval,
pendingTurnResults: make([]TurnResult, 0),
enableReminders: true,
enableAudioCues: true,
enableAutoBrowser: false, // Default to false for safety
browserRateLimiter: NewBrowserRateLimiter(browserOpenDelay, maxBrowserOpensMinute, maxBrowserOpensDay),
startTime: time.Now(),
}

// Load saved settings
Expand Down Expand Up @@ -492,6 +519,26 @@
app.checkForNewlyBlockedPRs(ctx)
}

// tryAutoOpenPR attempts to open a PR in the browser if enabled and rate limits allow.
func (app *App) tryAutoOpenPR(ctx context.Context, pr PR, autoBrowserEnabled bool, startTime time.Time) {
if !autoBrowserEnabled {
return
}

if app.browserRateLimiter.CanOpen(startTime, pr.URL) {
log.Printf("[BROWSER] Auto-opening newly blocked PR: %s #%d - %s",
pr.Repository, pr.Number, pr.URL)
// Use strict validation for auto-opened URLs
if err := openURLAutoStrict(ctx, pr.URL); err != nil {
log.Printf("[BROWSER] Failed to auto-open PR %s: %v", pr.URL, err)
} else {
app.browserRateLimiter.RecordOpen(pr.URL)
log.Printf("[BROWSER] Successfully opened PR %s #%d in browser",
pr.Repository, pr.Number)
}
}
}

// notifyWithSound sends a notification and plays sound only once per cycle.
func (app *App) notifyWithSound(ctx context.Context, pr PR, isIncoming bool, playedSound *bool) {
var title, soundType string
Expand Down Expand Up @@ -521,6 +568,8 @@
incoming := app.incoming
outgoing := app.outgoing
previousBlocked := app.previousBlockedPRs
autoBrowserEnabled := app.enableAutoBrowser
startTime := app.startTime
app.mu.RUnlock()

currentBlocked := make(map[string]bool)
Expand All @@ -534,6 +583,7 @@
// Notify if newly blocked
if !previousBlocked[incoming[i].URL] {
app.notifyWithSound(ctx, incoming[i], true, &playedHonk)
app.tryAutoOpenPR(ctx, incoming[i], autoBrowserEnabled, startTime)
}
}
}
Expand All @@ -549,6 +599,7 @@
time.Sleep(2 * time.Second)
}
app.notifyWithSound(ctx, outgoing[i], false, &playedJet)
app.tryAutoOpenPR(ctx, outgoing[i], autoBrowserEnabled, startTime)
}
}
}
Expand Down
56 changes: 56 additions & 0 deletions cmd/goose/security.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,11 @@
// New tokens: ghp_ (personal), ghs_ (server), ghr_ (refresh), gho_ (OAuth), ghu_ (user-to-server) followed by base62 chars.
// Fine-grained tokens: github_pat_ followed by base62 chars.
githubTokenRegex = regexp.MustCompile(`^[a-f0-9]{40}$|^gh[psoru]_[A-Za-z0-9]{36,251}$|^github_pat_[A-Za-z0-9]{82}$`)

// githubPRURLRegex validates strict GitHub PR URL format for auto-opening.
// Must match: https://github.com/{owner}/{repo}/pull/{number}
// Owner and repo follow GitHub naming rules, number is digits only.
githubPRURLRegex = regexp.MustCompile(`^https://github\.com/[a-zA-Z0-9][a-zA-Z0-9-]{0,38}/[a-zA-Z0-9][a-zA-Z0-9._-]{0,99}/pull/[1-9][0-9]{0,9}$`)
)

// validateGitHubUsername validates a GitHub username.
Expand Down Expand Up @@ -115,3 +120,54 @@

return nil
}

// validateGitHubPRURL performs strict validation for GitHub PR URLs used in auto-opening.
// This ensures the URL follows the exact pattern: https://github.com/{owner}/{repo}/pull/{number}
// with no additional path segments, fragments, or suspicious characters.
// The URL may optionally have ?goose=1 parameter which we add for tracking.
func validateGitHubPRURL(rawURL string) error {
// First do basic URL validation
if err := validateURL(rawURL); err != nil {
return err
}

// Strip the ?goose=1 parameter if present for pattern validation
urlToValidate := rawURL
if strings.HasSuffix(rawURL, "?goose=1") {
urlToValidate = strings.TrimSuffix(rawURL, "?goose=1")
}

// Check against strict GitHub PR URL pattern
if !githubPRURLRegex.MatchString(urlToValidate) {
return fmt.Errorf("URL does not match GitHub PR pattern: %s", urlToValidate)
}

// Additional security checks
// Reject URLs with @ (potential credential injection)
if strings.Contains(rawURL, "@") {
return errors.New("URL contains @ character")
}

// Reject URLs with URL encoding (could hide malicious content)

Check failure on line 151 in cmd/goose/security.go

View workflow job for this annotation

GitHub Actions / golangci-lint

File is not properly formatted (gofumpt)
// Exception: %3D which is = in URL encoding, only as part of ?goose=1
if strings.Contains(rawURL, "%") && !strings.HasSuffix(rawURL, "?goose%3D1") {
return errors.New("URL contains encoded characters")
}

// Reject URLs with fragments
if strings.Contains(rawURL, "#") {
return errors.New("URL contains fragments")
}

// Allow only ?goose=1 query parameter, nothing else
if strings.Contains(rawURL, "?") && !strings.HasSuffix(rawURL, "?goose=1") && !strings.HasSuffix(rawURL, "?goose%3D1") {
return errors.New("URL contains unexpected query parameters")
}

// Reject URLs with double slashes (except after https:)
if strings.Contains(rawURL[8:], "//") {
return errors.New("URL contains double slashes")
}

return nil
}
Loading
Loading