Skip to content

Commit bfcb792

Browse files
Deeven-Seruduwenxin99
authored andcommitted
security(http): prevent base URL host override (googleapis#2655)
## Summary - build request URLs by resolving validated relative paths against the base URL - reject path templates that attempt to set scheme/host/userinfo - add regression tests for host-override attempts ## Testing - go test ./internal/tools/http Fixes googleapis#2616 Co-authored-by: Wenxin Du <117315983+duwenxin99@users.noreply.github.com>
1 parent dcf0394 commit bfcb792

2 files changed

Lines changed: 96 additions & 4 deletions

File tree

internal/tools/http/http.go

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -175,12 +175,26 @@ func getURL(baseURL, path string, pathParams, queryParams parameters.Parameters,
175175
return "", fmt.Errorf("error replacing pathParams: %s", err)
176176
}
177177

178-
// Create URL based on BaseURL and Path
179-
// Attach query parameters
180-
parsedURL, err := url.Parse(baseURL + templatedPath.String())
178+
baseParsedURL, err := url.Parse(baseURL)
181179
if err != nil {
182-
return "", fmt.Errorf("error parsing URL: %s", err)
180+
return "", fmt.Errorf("error parsing base URL: %s", err)
183181
}
182+
if baseParsedURL.Scheme == "" || baseParsedURL.Host == "" {
183+
return "", fmt.Errorf("base URL must include scheme and host")
184+
}
185+
186+
relativePath := templatedPath.String()
187+
relParsedURL, err := url.Parse(relativePath)
188+
if err != nil {
189+
return "", fmt.Errorf("error parsing URL path: %s", err)
190+
}
191+
if relParsedURL.Scheme != "" || relParsedURL.Host != "" || relParsedURL.User != nil {
192+
return "", fmt.Errorf("path must be relative and cannot override base host")
193+
}
194+
195+
// Create URL based on BaseURL and Path
196+
// Attach query parameters
197+
parsedURL := baseParsedURL.ResolveReference(relParsedURL)
184198

185199
// Get existing query parameters from the URL
186200
queryParameters := parsedURL.Query()
Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
// Copyright 2026 Google LLC
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
package http
16+
17+
import (
18+
"net/url"
19+
"testing"
20+
21+
"github.com/googleapis/genai-toolbox/internal/util/parameters"
22+
)
23+
24+
func TestGetURLHostOverride(t *testing.T) {
25+
testCases := []struct {
26+
name string
27+
pathParam string
28+
expectError bool
29+
expectedHost string
30+
}{
31+
{
32+
name: "at sign in path is not a host override",
33+
pathParam: "@evil.com/v1",
34+
expectError: false,
35+
expectedHost: "api.good.com",
36+
},
37+
{
38+
name: "absolute url in path is rejected",
39+
pathParam: "https://evil.com/v1",
40+
expectError: true,
41+
},
42+
{
43+
name: "authority override in path is rejected",
44+
pathParam: "//evil.com/v1",
45+
expectError: true,
46+
},
47+
}
48+
49+
baseURL := "https://api.good.com"
50+
path := "{{.pathParam}}"
51+
pathParams := parameters.Parameters{parameters.NewStringParameter("pathParam", "path")}
52+
53+
for _, tc := range testCases {
54+
t.Run(tc.name, func(t *testing.T) {
55+
paramsMap := map[string]any{"pathParam": tc.pathParam}
56+
57+
urlString, err := getURL(baseURL, path, pathParams, nil, nil, paramsMap)
58+
if tc.expectError {
59+
if err == nil {
60+
t.Fatalf("expected error but got none")
61+
}
62+
return
63+
}
64+
if err != nil {
65+
t.Fatalf("unexpected error: %v", err)
66+
}
67+
68+
parsed, err := url.Parse(urlString)
69+
if err != nil {
70+
t.Fatalf("failed to parse URL: %v", err)
71+
}
72+
73+
if parsed.Host != tc.expectedHost {
74+
t.Fatalf("expected host to be %q, got %q", tc.expectedHost, parsed.Host)
75+
}
76+
})
77+
}
78+
}

0 commit comments

Comments
 (0)