Skip to content

Commit 71e80d5

Browse files
tstrombergtstromberg
authored
Merge pull request #85 from codeGROOVE-dev/icons
upgrade sprinkler
2 parents aac3c5c + 69ae844 commit 71e80d5

File tree

6 files changed

+215
-447
lines changed

6 files changed

+215
-447
lines changed

‎cmd/goose/security.go‎

Lines changed: 0 additions & 97 deletions
Original file line numberDiff line numberDiff line change
@@ -28,11 +28,6 @@ var (
2828
// New tokens: ghp_ (personal), ghs_ (server), ghr_ (refresh), gho_ (OAuth), ghu_ (user-to-server) followed by base62 chars.
2929
// Fine-grained tokens: github_pat_ followed by base62 chars.
3030
githubTokenRegex = regexp.MustCompile(`^[a-f0-9]{40}$|^gh[psoru]_[A-Za-z0-9]{36,251}$|^github_pat_[A-Za-z0-9]{82}$`)
31-
32-
// githubPRURLRegex validates strict GitHub PR URL format for auto-opening.
33-
// Must match: https://github.com/{owner}/{repo}/pull/{number}
34-
// Owner and repo follow GitHub naming rules, number is digits only.
35-
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}$`)
3631
)
3732

3833
// validateGitHubUsername validates a GitHub username.
@@ -93,95 +88,3 @@ func sanitizeForLog(s string) string {
9388

9489
return s
9590
}
96-
97-
// validateURL performs strict validation on URLs.
98-
func validateURL(rawURL string) error {
99-
if rawURL == "" {
100-
return errors.New("URL cannot be empty")
101-
}
102-
103-
// Check for null bytes or control characters
104-
for _, r := range rawURL {
105-
if r < minPrintableChar || r == deleteChar {
106-
return errors.New("URL contains control characters")
107-
}
108-
}
109-
110-
// Ensure URL starts with https://
111-
if !strings.HasPrefix(rawURL, "https://") {
112-
return errors.New("URL must use HTTPS")
113-
}
114-
115-
// Check for URL length limits
116-
if len(rawURL) > maxURLLength {
117-
return errors.New("URL too long")
118-
}
119-
120-
return nil
121-
}
122-
123-
// validateGitHubPRURL performs strict validation for GitHub PR URLs used in auto-opening.
124-
// This ensures the URL follows the exact pattern: https://github.com/{owner}/{repo}/pull/{number}
125-
// with no additional path segments, fragments, or suspicious characters.
126-
// The URL may optionally have ?goose=<action> parameter which we add for tracking.
127-
func validateGitHubPRURL(rawURL string) error {
128-
// First do basic URL validation
129-
if err := validateURL(rawURL); err != nil {
130-
return err
131-
}
132-
133-
// Strip the ?goose parameter if present for pattern validation
134-
urlToValidate := rawURL
135-
if idx := strings.Index(rawURL, "?goose="); idx != -1 {
136-
urlToValidate = rawURL[:idx]
137-
}
138-
139-
// Check against strict GitHub PR URL pattern
140-
if !githubPRURLRegex.MatchString(urlToValidate) {
141-
return fmt.Errorf("URL does not match GitHub PR pattern: %s", urlToValidate)
142-
}
143-
144-
// Additional security checks
145-
// Reject URLs with @ (potential credential injection)
146-
if strings.Contains(rawURL, "@") {
147-
return errors.New("URL contains @ character")
148-
}
149-
150-
// Reject URLs with URL encoding (could hide malicious content)
151-
// Exception: %3D which is = in URL encoding, only as part of ?goose parameter
152-
if strings.Contains(rawURL, "%") {
153-
// Allow URL encoding only in the goose parameter value
154-
idx := strings.Index(rawURL, "?goose=")
155-
if idx == -1 {
156-
return errors.New("URL contains encoded characters")
157-
}
158-
// Check if encoding is only in the goose parameter
159-
if strings.Contains(rawURL[:idx], "%") {
160-
return errors.New("URL contains encoded characters outside goose parameter")
161-
}
162-
}
163-
164-
// Reject URLs with fragments
165-
if strings.Contains(rawURL, "#") {
166-
return errors.New("URL contains fragments")
167-
}
168-
169-
// Allow only ?goose=<value> query parameter, nothing else
170-
if strings.Contains(rawURL, "?") {
171-
// Check if it's the goose parameter
172-
if idx := strings.Index(rawURL, "?goose="); idx == -1 {
173-
return errors.New("URL contains unexpected query parameters")
174-
}
175-
// Ensure no additional parameters after goose
176-
if strings.Contains(rawURL[strings.Index(rawURL, "?goose=")+7:], "&") {
177-
return errors.New("URL contains additional query parameters")
178-
}
179-
}
180-
181-
// Reject URLs with double slashes (except after https:)
182-
if strings.Contains(rawURL[8:], "//") {
183-
return errors.New("URL contains double slashes")
184-
}
185-
186-
return nil
187-
}

‎cmd/goose/security_test.go‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -261,11 +261,9 @@ func TestSanitizeForLog(t *testing.T) {
261261
if strings.Contains(result, "ghp_") || strings.Contains(result, "github_pat_") {
262262
t.Errorf("sanitizeForLog() = %v, still contains sensitive pattern", result)
263263
}
264-
} else {
264+
} else if result != tt.input {
265265
// Should be unchanged
266-
if result != tt.input {
267-
t.Errorf("sanitizeForLog() = %v, want %v", result, tt.input)
268-
}
266+
t.Errorf("sanitizeForLog() = %v, want %v", result, tt.input)
269267
}
270268
})
271269
}

‎go.mod‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ go 1.24.0
44

55
require (
66
github.com/codeGROOVE-dev/retry v1.3.0
7-
github.com/codeGROOVE-dev/sprinkler v0.0.0-20251105232821-c5aeed50a046
7+
github.com/codeGROOVE-dev/sprinkler v0.0.0-20251113030909-5962af625370
88
github.com/codeGROOVE-dev/turnclient v0.0.0-20251107215141-ee43672b3dc7
99
github.com/energye/systray v1.0.2
1010
github.com/gen2brain/beeep v0.11.1

‎go.sum‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,8 @@ github.com/codeGROOVE-dev/prx v0.0.0-20251109164430-90488144076d h1:KKt93PVYR9Ug
44
github.com/codeGROOVE-dev/prx v0.0.0-20251109164430-90488144076d/go.mod h1:FEy3gz9IYDXWnKWkoDSL+pWu6rujxbBSrF4w5A8QSK0=
55
github.com/codeGROOVE-dev/retry v1.3.0 h1:/+ipAWRJLL6y1R1vprYo0FSjSBvH6fE5j9LKXjpD54g=
66
github.com/codeGROOVE-dev/retry v1.3.0/go.mod h1:8OgefgV1XP7lzX2PdKlCXILsYKuz6b4ZpHa/20iLi8E=
7-
github.com/codeGROOVE-dev/sprinkler v0.0.0-20251105232821-c5aeed50a046 h1:OsTP4XobXlDHrCD3ClX3W0Uhxay52wRutzqv5wEygdc=
8-
github.com/codeGROOVE-dev/sprinkler v0.0.0-20251105232821-c5aeed50a046/go.mod h1:/kd3ncsRNldD0MUpbtp5ojIzfCkyeXB7JdOrpuqG7Gg=
7+
github.com/codeGROOVE-dev/sprinkler v0.0.0-20251113030909-5962af625370 h1:uYXBDnaRRf4q6X/IWOm6O/cOj57tVkpjfVvwn+SfHp0=
8+
github.com/codeGROOVE-dev/sprinkler v0.0.0-20251113030909-5962af625370/go.mod h1:sOvKRad1kRPAOIUm7spNR3aeVQjtk9VoS8uo6NL4kus=
99
github.com/codeGROOVE-dev/turnclient v0.0.0-20251107215141-ee43672b3dc7 h1:183q0bj2y/9hh/K0HZvDXI6sG7liYSRcQVgFx0GY+UA=
1010
github.com/codeGROOVE-dev/turnclient v0.0.0-20251107215141-ee43672b3dc7/go.mod h1:dVS3MlJDgL6WkfurJAyS7I9Fe1yxxoxxarjVifY5bIo=
1111
github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=

‎pkg/safebrowse/safebrowse.go‎

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ func Open(ctx context.Context, rawURL string) error {
1919
if err := validate(rawURL, false); err != nil {
2020
return err
2121
}
22-
return open(ctx, rawURL)
22+
return openBrowser(ctx, rawURL)
2323
}
2424

2525
// OpenWithParams validates and opens a URL with query parameters.
@@ -60,7 +60,7 @@ func OpenWithParams(ctx context.Context, rawURL string, params map[string]string
6060
return err
6161
}
6262

63-
return open(ctx, finalURL)
63+
return openBrowser(ctx, finalURL)
6464
}
6565

6666
// ValidateURL performs strict security validation on a URL.
@@ -75,7 +75,11 @@ func ValidateGitHubPRURL(rawURL string) error {
7575
}
7676

7777
// Additional GitHub PR-specific checks
78-
u, _ := url.Parse(rawURL) // Already validated
78+
u, err := url.Parse(rawURL)
79+
if err != nil {
80+
// Should never happen since we already validated, but check anyway
81+
return fmt.Errorf("parse url: %w", err)
82+
}
7983
if u.Host != "github.com" {
8084
return errors.New("must be github.com")
8185
}
@@ -86,7 +90,7 @@ func ValidateGitHubPRURL(rawURL string) error {
8690
}
8791

8892
// Validate PR number (must start with 1-9)
89-
if len(parts[3]) == 0 || parts[3][0] < '1' || parts[3][0] > '9' {
93+
if parts[3] == "" || parts[3][0] < '1' || parts[3][0] > '9' {
9094
return errors.New("PR number must start with 1-9")
9195
}
9296
for _, c := range parts[3] {
@@ -200,15 +204,15 @@ func validateParamString(s string) error {
200204
return errors.New("cannot be empty")
201205
}
202206
for _, r := range s {
203-
if !((r >= 'a' && r <= 'z') || (r >= 'A' && r <= 'Z') || (r >= '0' && r <= '9') || r == '-' || r == '_') {
207+
if (r < 'a' || r > 'z') && (r < 'A' || r > 'Z') && (r < '0' || r > '9') && r != '-' && r != '_' {
204208
return fmt.Errorf("contains invalid character %q", r)
205209
}
206210
}
207211
return nil
208212
}
209213

210-
// open opens a URL in the system browser.
211-
func open(ctx context.Context, rawURL string) error {
214+
// openBrowser opens a URL in the system browser.
215+
func openBrowser(ctx context.Context, rawURL string) error {
212216
var cmd *exec.Cmd
213217

214218
switch runtime.GOOS {

0 commit comments

Comments
 (0)