Skip to content

Commit d17891b

Browse files
tstrombergtstromberg
committed
Add missing files for the auto-opener
1 parent bc194f2 commit d17891b

File tree

2 files changed

+227
-0
lines changed

2 files changed

+227
-0
lines changed

‎cmd/goose/browser_rate_limiter.go‎

Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,119 @@
1+
// Package main implements browser rate limiting for PR auto-open feature.
2+
package main
3+
4+
import (
5+
"log"
6+
"sync"
7+
"time"
8+
)
9+
10+
// BrowserRateLimiter manages rate limiting for automatically opening browser windows.
11+
type BrowserRateLimiter struct {
12+
openedPRs map[string]bool
13+
openedLastMinute []time.Time
14+
openedToday []time.Time
15+
startupDelay time.Duration
16+
maxPerMinute int
17+
maxPerDay int
18+
mu sync.Mutex
19+
}
20+
21+
// NewBrowserRateLimiter creates a new browser rate limiter.
22+
func NewBrowserRateLimiter(startupDelay time.Duration, maxPerMinute, maxPerDay int) *BrowserRateLimiter {
23+
log.Printf("[BROWSER] Initializing rate limiter: startup_delay=%v, max_per_minute=%d, max_per_day=%d",
24+
startupDelay, maxPerMinute, maxPerDay)
25+
return &BrowserRateLimiter{
26+
openedLastMinute: make([]time.Time, 0),
27+
openedToday: make([]time.Time, 0),
28+
startupDelay: startupDelay,
29+
maxPerMinute: maxPerMinute,
30+
maxPerDay: maxPerDay,
31+
openedPRs: make(map[string]bool),
32+
}
33+
}
34+
35+
// CanOpen checks if we can open a browser window according to rate limits.
36+
func (b *BrowserRateLimiter) CanOpen(startTime time.Time, prURL string) bool {
37+
b.mu.Lock()
38+
defer b.mu.Unlock()
39+
40+
// Check if we've already opened this PR
41+
if b.openedPRs[prURL] {
42+
log.Printf("[BROWSER] Skipping auto-open: PR already opened - %s", prURL)
43+
return false
44+
}
45+
46+
// Check startup delay
47+
if time.Since(startTime) < b.startupDelay {
48+
log.Printf("[BROWSER] Skipping auto-open: within startup delay period (%v remaining)",
49+
b.startupDelay-time.Since(startTime))
50+
return false
51+
}
52+
53+
now := time.Now()
54+
55+
// Clean old entries
56+
b.cleanOldEntries(now)
57+
58+
// Check per-minute limit
59+
if len(b.openedLastMinute) >= b.maxPerMinute {
60+
log.Printf("[BROWSER] Rate limit: already opened %d PRs in the last minute (max: %d)",
61+
len(b.openedLastMinute), b.maxPerMinute)
62+
return false
63+
}
64+
65+
// Check per-day limit
66+
if len(b.openedToday) >= b.maxPerDay {
67+
log.Printf("[BROWSER] Rate limit: already opened %d PRs today (max: %d)",
68+
len(b.openedToday), b.maxPerDay)
69+
return false
70+
}
71+
72+
return true
73+
}
74+
75+
// RecordOpen records that a browser window was opened.
76+
func (b *BrowserRateLimiter) RecordOpen(prURL string) {
77+
b.mu.Lock()
78+
defer b.mu.Unlock()
79+
80+
now := time.Now()
81+
b.openedLastMinute = append(b.openedLastMinute, now)
82+
b.openedToday = append(b.openedToday, now)
83+
b.openedPRs[prURL] = true
84+
85+
log.Printf("[BROWSER] Recorded browser open for %s (minute: %d/%d, today: %d/%d)",
86+
prURL, len(b.openedLastMinute), b.maxPerMinute, len(b.openedToday), b.maxPerDay)
87+
}
88+
89+
// cleanOldEntries removes entries outside the time windows.
90+
func (b *BrowserRateLimiter) cleanOldEntries(now time.Time) {
91+
// Clean entries older than 1 minute
92+
oneMinuteAgo := now.Add(-1 * time.Minute)
93+
newLastMinute := make([]time.Time, 0, len(b.openedLastMinute))
94+
for _, t := range b.openedLastMinute {
95+
if t.After(oneMinuteAgo) {
96+
newLastMinute = append(newLastMinute, t)
97+
}
98+
}
99+
b.openedLastMinute = newLastMinute
100+
101+
// Clean entries older than 24 hours (1 day)
102+
oneDayAgo := now.Add(-24 * time.Hour)
103+
newToday := make([]time.Time, 0, len(b.openedToday))
104+
for _, t := range b.openedToday {
105+
if t.After(oneDayAgo) {
106+
newToday = append(newToday, t)
107+
}
108+
}
109+
b.openedToday = newToday
110+
}
111+
112+
// Reset clears the opened PRs tracking - useful when toggling the feature.
113+
func (b *BrowserRateLimiter) Reset() {
114+
b.mu.Lock()
115+
defer b.mu.Unlock()
116+
previousCount := len(b.openedPRs)
117+
b.openedPRs = make(map[string]bool)
118+
log.Printf("[BROWSER] Rate limiter reset: cleared %d tracked PRs", previousCount)
119+
}

‎cmd/goose/security_test.go‎

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
package main
2+
3+
import (
4+
"testing"
5+
)
6+
7+
func TestValidateGitHubPRURL(t *testing.T) {
8+
tests := []struct {
9+
name string
10+
url string
11+
wantErr bool
12+
}{
13+
// Valid URLs
14+
{
15+
name: "valid PR URL",
16+
url: "https://github.com/owner/repo/pull/123",
17+
wantErr: false,
18+
},
19+
{
20+
name: "valid PR URL with goose param",
21+
url: "https://github.com/owner/repo/pull/123?goose=1",
22+
wantErr: false,
23+
},
24+
{
25+
name: "valid PR URL with hyphen in owner",
26+
url: "https://github.com/owner-name/repo/pull/1",
27+
wantErr: false,
28+
},
29+
{
30+
name: "valid PR URL with dots in repo",
31+
url: "https://github.com/owner/repo.name/pull/999",
32+
wantErr: false,
33+
},
34+
35+
// Invalid URLs - security issues
36+
{
37+
name: "URL with credential injection",
38+
url: "https://[email protected]/owner/repo/pull/123",
39+
wantErr: true,
40+
},
41+
{
42+
name: "URL with encoded characters",
43+
url: "https://github.com/owner/repo/pull/123%2F../",
44+
wantErr: true,
45+
},
46+
{
47+
name: "URL with double slashes",
48+
url: "https://github.com//owner/repo/pull/123",
49+
wantErr: true,
50+
},
51+
{
52+
name: "URL with fragment",
53+
url: "https://github.com/owner/repo/pull/123#evil",
54+
wantErr: true,
55+
},
56+
{
57+
name: "URL with extra query params",
58+
url: "https://github.com/owner/repo/pull/123?foo=bar",
59+
wantErr: true,
60+
},
61+
{
62+
name: "URL with extra path segments",
63+
url: "https://github.com/owner/repo/pull/123/files",
64+
wantErr: true,
65+
},
66+
67+
// Invalid URLs - wrong format
68+
{
69+
name: "not a PR URL",
70+
url: "https://github.com/owner/repo",
71+
wantErr: true,
72+
},
73+
{
74+
name: "issue URL instead of PR",
75+
url: "https://github.com/owner/repo/issues/123",
76+
wantErr: true,
77+
},
78+
{
79+
name: "HTTP instead of HTTPS",
80+
url: "http://github.com/owner/repo/pull/123",
81+
wantErr: true,
82+
},
83+
{
84+
name: "wrong domain",
85+
url: "https://gitlab.com/owner/repo/pull/123",
86+
wantErr: true,
87+
},
88+
{
89+
name: "PR number with leading zero",
90+
url: "https://github.com/owner/repo/pull/0123",
91+
wantErr: true,
92+
},
93+
{
94+
name: "PR number zero",
95+
url: "https://github.com/owner/repo/pull/0",
96+
wantErr: true,
97+
},
98+
}
99+
100+
for _, tt := range tests {
101+
t.Run(tt.name, func(t *testing.T) {
102+
err := validateGitHubPRURL(tt.url)
103+
if (err != nil) != tt.wantErr {
104+
t.Errorf("validateGitHubPRURL() error = %v, wantErr %v", err, tt.wantErr)
105+
}
106+
})
107+
}
108+
}

0 commit comments

Comments
 (0)