Skip to content

Commit 53e8875

Browse files
chmouelchmouel
committed
fix: add URL validation for LLM providers
Add validation for BaseURL configuration in both Gemini and OpenAI LLM provider clients to prevent invalid URLs from being used. The ValidateBaseURL helper function validates: - URL scheme (must be http or https) - URL has a valid host - URL contains no invalid whitespace characters This addresses review comments requesting URL validation. Signed-off-by: Chmouel Boudjnah <[email protected]> Assisted-by: Claude-3.5-Sonnet (via Cursor)
1 parent c49eb10 commit 53e8875

File tree

6 files changed

+229
-2
lines changed

6 files changed

+229
-2
lines changed

‎pkg/llm/providers/common.go‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@ package providers
33

44
import (
55
"fmt"
6+
"net/url"
7+
"strings"
68
)
79

810
// CommonConfig represents the common configuration fields across all LLM providers.
@@ -61,3 +63,30 @@ func ApplyDefaults(config any) error {
6163

6264
return nil
6365
}
66+
67+
// ValidateBaseURL validates that the provided baseURL is a valid HTTP/HTTPS URL.
68+
func ValidateBaseURL(baseURL string) error {
69+
if baseURL == "" {
70+
return fmt.Errorf("base URL is required")
71+
}
72+
73+
parsedURL, err := url.Parse(baseURL)
74+
if err != nil {
75+
return fmt.Errorf("invalid base URL: %w", err)
76+
}
77+
78+
if parsedURL.Scheme != "http" && parsedURL.Scheme != "https" {
79+
return fmt.Errorf("base URL must use http or https scheme, got: %s", parsedURL.Scheme)
80+
}
81+
82+
if parsedURL.Host == "" {
83+
return fmt.Errorf("base URL must have a valid host")
84+
}
85+
86+
// Check for invalid characters that might cause issues
87+
if strings.ContainsAny(baseURL, " \t\n\r") {
88+
return fmt.Errorf("base URL contains invalid whitespace characters")
89+
}
90+
91+
return nil
92+
}

‎pkg/llm/providers/common_test.go‎

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -223,3 +223,89 @@ func TestValidateAndApplyDefaults(t *testing.T) {
223223
assert.Equal(t, config.TimeoutSeconds, 30)
224224
assert.Equal(t, config.MaxTokens, 1000)
225225
}
226+
227+
func TestValidateBaseURL(t *testing.T) {
228+
tests := []struct {
229+
name string
230+
baseURL string
231+
wantErr string
232+
}{
233+
{
234+
name: "empty URL",
235+
baseURL: "",
236+
wantErr: "base URL is required",
237+
},
238+
{
239+
name: "valid HTTPS URL",
240+
baseURL: "https://api.example.com",
241+
wantErr: "",
242+
},
243+
{
244+
name: "valid HTTP URL",
245+
baseURL: "http://api.example.com",
246+
wantErr: "",
247+
},
248+
{
249+
name: "valid URL with port",
250+
baseURL: "https://api.example.com:8443",
251+
wantErr: "",
252+
},
253+
{
254+
name: "valid URL with path",
255+
baseURL: "https://api.example.com/v1",
256+
wantErr: "",
257+
},
258+
{
259+
name: "invalid URL - no scheme",
260+
baseURL: "api.example.com",
261+
wantErr: "base URL must use http or https scheme",
262+
},
263+
{
264+
name: "invalid URL - wrong scheme (ftp)",
265+
baseURL: "ftp://api.example.com",
266+
wantErr: "base URL must use http or https scheme",
267+
},
268+
{
269+
name: "invalid URL - wrong scheme (ws)",
270+
baseURL: "ws://api.example.com",
271+
wantErr: "base URL must use http or https scheme",
272+
},
273+
{
274+
name: "invalid URL - no host",
275+
baseURL: "https://",
276+
wantErr: "base URL must have a valid host",
277+
},
278+
{
279+
name: "invalid URL - with whitespace",
280+
baseURL: "https://api.example.com /path",
281+
wantErr: "invalid base URL",
282+
},
283+
{
284+
name: "invalid URL - with tab",
285+
baseURL: "https://api.example.com\t/path",
286+
wantErr: "invalid base URL",
287+
},
288+
{
289+
name: "invalid URL - with newline",
290+
baseURL: "https://api.example.com\n",
291+
wantErr: "invalid base URL",
292+
},
293+
{
294+
name: "invalid URL - malformed",
295+
baseURL: "ht!tp://api.example.com",
296+
wantErr: "invalid base URL",
297+
},
298+
}
299+
300+
for _, tt := range tests {
301+
t.Run(tt.name, func(t *testing.T) {
302+
err := ValidateBaseURL(tt.baseURL)
303+
if tt.wantErr == "" {
304+
assert.NilError(t, err)
305+
} else {
306+
assert.Assert(t, err != nil)
307+
assert.ErrorContains(t, err, tt.wantErr)
308+
}
309+
})
310+
}
311+
}

‎pkg/llm/providers/gemini/client.go‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -240,7 +240,16 @@ func (c *Client) ValidateConfig() error {
240240
TimeoutSeconds: c.config.TimeoutSeconds,
241241
MaxTokens: c.config.MaxTokens,
242242
}
243-
return providers.ValidateCommonConfig(commonCfg)
243+
if err := providers.ValidateCommonConfig(commonCfg); err != nil {
244+
return err
245+
}
246+
247+
// Validate BaseURL
248+
if err := providers.ValidateBaseURL(c.config.BaseURL); err != nil {
249+
return err
250+
}
251+
252+
return nil
244253
}
245254

246255
// buildPrompt combines the base prompt with context data.

‎pkg/llm/providers/gemini/client_test.go‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,7 @@ func TestValidateConfig(t *testing.T) {
8888
name: "valid config",
8989
config: &Config{
9090
APIKey: "valid-key",
91+
BaseURL: "https://api.example.com",
9192
TimeoutSeconds: 30,
9293
MaxTokens: 1000,
9394
},
@@ -107,6 +108,7 @@ func TestValidateConfig(t *testing.T) {
107108
name: "negative timeout",
108109
config: &Config{
109110
APIKey: "valid-key",
111+
BaseURL: "https://api.example.com",
110112
TimeoutSeconds: -1,
111113
MaxTokens: 1000,
112114
},
@@ -117,12 +119,57 @@ func TestValidateConfig(t *testing.T) {
117119
name: "negative max tokens",
118120
config: &Config{
119121
APIKey: "valid-key",
122+
BaseURL: "https://api.example.com",
120123
TimeoutSeconds: 30,
121124
MaxTokens: -1,
122125
},
123126
wantError: true,
124127
errMsg: "max tokens must be non-negative",
125128
},
129+
{
130+
name: "invalid URL - no scheme",
131+
config: &Config{
132+
APIKey: "valid-key",
133+
BaseURL: "api.example.com",
134+
TimeoutSeconds: 30,
135+
MaxTokens: 1000,
136+
},
137+
wantError: true,
138+
errMsg: "base URL must use http or https scheme",
139+
},
140+
{
141+
name: "invalid URL - wrong scheme",
142+
config: &Config{
143+
APIKey: "valid-key",
144+
BaseURL: "ftp://api.example.com",
145+
TimeoutSeconds: 30,
146+
MaxTokens: 1000,
147+
},
148+
wantError: true,
149+
errMsg: "base URL must use http or https scheme",
150+
},
151+
{
152+
name: "invalid URL - has whitespace",
153+
config: &Config{
154+
APIKey: "valid-key",
155+
BaseURL: "https://api.example.com /path",
156+
TimeoutSeconds: 30,
157+
MaxTokens: 1000,
158+
},
159+
wantError: true,
160+
errMsg: "invalid base URL",
161+
},
162+
{
163+
name: "invalid URL - no host",
164+
config: &Config{
165+
APIKey: "valid-key",
166+
BaseURL: "https://",
167+
TimeoutSeconds: 30,
168+
MaxTokens: 1000,
169+
},
170+
wantError: true,
171+
errMsg: "base URL must have a valid host",
172+
},
126173
}
127174

128175
for _, tt := range tests {

‎pkg/llm/providers/openai/client.go‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -231,7 +231,16 @@ func (c *Client) ValidateConfig() error {
231231
TimeoutSeconds: c.config.TimeoutSeconds,
232232
MaxTokens: c.config.MaxTokens,
233233
}
234-
return providers.ValidateCommonConfig(commonCfg)
234+
if err := providers.ValidateCommonConfig(commonCfg); err != nil {
235+
return err
236+
}
237+
238+
// Validate BaseURL
239+
if err := providers.ValidateBaseURL(c.config.BaseURL); err != nil {
240+
return err
241+
}
242+
243+
return nil
235244
}
236245

237246
// buildPrompt combines the base prompt with context data.

‎pkg/llm/providers/openai/client_test.go‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,7 @@ func TestValidateConfig(t *testing.T) {
104104
name: "valid config",
105105
config: &Config{
106106
APIKey: "valid-key",
107+
BaseURL: "https://api.openai.com/v1",
107108
TimeoutSeconds: 30,
108109
MaxTokens: 1000,
109110
},
@@ -123,6 +124,7 @@ func TestValidateConfig(t *testing.T) {
123124
name: "negative timeout",
124125
config: &Config{
125126
APIKey: "valid-key",
127+
BaseURL: "https://api.openai.com/v1",
126128
TimeoutSeconds: -1,
127129
MaxTokens: 1000,
128130
},
@@ -133,12 +135,57 @@ func TestValidateConfig(t *testing.T) {
133135
name: "negative max tokens",
134136
config: &Config{
135137
APIKey: "valid-key",
138+
BaseURL: "https://api.openai.com/v1",
136139
TimeoutSeconds: 30,
137140
MaxTokens: -1,
138141
},
139142
wantError: true,
140143
errMsg: "max tokens must be non-negative",
141144
},
145+
{
146+
name: "invalid URL - no scheme",
147+
config: &Config{
148+
APIKey: "valid-key",
149+
BaseURL: "api.openai.com",
150+
TimeoutSeconds: 30,
151+
MaxTokens: 1000,
152+
},
153+
wantError: true,
154+
errMsg: "base URL must use http or https scheme",
155+
},
156+
{
157+
name: "invalid URL - wrong scheme",
158+
config: &Config{
159+
APIKey: "valid-key",
160+
BaseURL: "ftp://api.openai.com",
161+
TimeoutSeconds: 30,
162+
MaxTokens: 1000,
163+
},
164+
wantError: true,
165+
errMsg: "base URL must use http or https scheme",
166+
},
167+
{
168+
name: "invalid URL - has whitespace",
169+
config: &Config{
170+
APIKey: "valid-key",
171+
BaseURL: "https://api.openai.com /v1",
172+
TimeoutSeconds: 30,
173+
MaxTokens: 1000,
174+
},
175+
wantError: true,
176+
errMsg: "invalid base URL",
177+
},
178+
{
179+
name: "invalid URL - no host",
180+
config: &Config{
181+
APIKey: "valid-key",
182+
BaseURL: "https://",
183+
TimeoutSeconds: 30,
184+
MaxTokens: 1000,
185+
},
186+
wantError: true,
187+
errMsg: "base URL must have a valid host",
188+
},
142189
}
143190

144191
for _, tt := range tests {

0 commit comments

Comments
 (0)