Skip to content

Commit f07ab8c

Browse files
authored
fix(storage): remove dead DelegateRequest to resolve CodeQL SSRF alert (#228)
* fix(storage): remove dead DelegateRequest to resolve CodeQL SSRF alert DelegateRequest and its helper mapURL were never called from production code — the intended caller (forwardRequest) was commented out at introduction in 2017 and deleted in 2019. Removing them eliminates the taint flow that CodeQL alert #15 (go/request-forgery) tracks, since no user-controlled URL now reaches sendToPrometheus. * fix(storage): narrow comment to Driver methods per review feedback
1 parent 18ce851 commit f07ab8c

2 files changed

Lines changed: 7 additions & 26 deletions

File tree

pkg/storage/interface.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,6 @@ type Driver interface {
137137
Series(match []string, start, end string, acceptContentType string) (*http.Response, error)
138138
LabelValues(name string, acceptContentType string) (*http.Response, error)
139139
Labels(start, end string, match []string, acceptContentType string) (*http.Response, error)
140-
DelegateRequest(request *http.Request) (*http.Response, error)
141140
}
142141

143142
// NewPrometheusDriver is a factory method which chooses the right driver implementation based on configuration settings

pkg/storage/prometheus.go

Lines changed: 7 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -103,12 +103,6 @@ func (promCli *prometheusStorageClient) Federate(selectors []string, acceptConte
103103
return promCli.sendToPrometheus("GET", promURL.String(), nil, map[string]string{"Accept": acceptContentType})
104104
}
105105

106-
func (promCli *prometheusStorageClient) DelegateRequest(request *http.Request) (*http.Response, error) {
107-
promURL := promCli.mapURL(request.URL)
108-
109-
return promCli.sendToPrometheus(request.Method, promURL.String(), request.Body, map[string]string{"Accept": request.Header.Get("Accept")})
110-
}
111-
112106
// buildURL is used to build the target URL of a Prometheus call
113107
func (promCli *prometheusStorageClient) buildURL(path string, params map[string]any) url.URL {
114108
promURL := *promCli.url
@@ -136,26 +130,14 @@ func (promCli *prometheusStorageClient) buildURL(path string, params map[string]
136130
return promURL
137131
}
138132

139-
// mapURL is used to map a Maia URL to Prometheus URL
140-
func (promCli *prometheusStorageClient) mapURL(maiaURL *url.URL) url.URL {
141-
promURL := *maiaURL
142-
143-
// change original request to point to our backing Prometheus
144-
promURL.Host = promCli.url.Host
145-
promURL.Scheme = promCli.url.Scheme
146-
promURL.User = promCli.url.User
147-
promURL.RawQuery = ""
148-
149-
return promURL
150-
}
151-
152-
// SendToPrometheus takes care of the request wrapping and delivery to Prometheus
133+
// sendToPrometheus takes care of the request wrapping and delivery to Prometheus.
134+
//
135+
//nolint:unparam // method is currently always "GET" but kept generic for API flexibility
153136
func (promCli *prometheusStorageClient) sendToPrometheus(method, promURL string, body io.Reader, headers map[string]string) (*http.Response, error) {
154-
// Validate the URL before proceeding with the request. This is defense-in-depth
155-
// against CodeQL go/request-forgery: mapURL() already overwrites Scheme/Host/User
156-
// to the trusted upstream, but a future change to mapURL() could regress that
157-
// invariant. Rejecting any host other than the configured upstream ensures the
158-
// request never leaves the process for a foreign destination.
137+
// Defense-in-depth: verify the URL targets a trusted upstream before sending.
138+
// All Driver methods construct URLs via buildURL() which uses only the
139+
// configured promCli.url / promCli.federateURL base, but this check makes
140+
// the safety property explicit and guards against future regressions.
159141
if err := promCli.validateUpstreamURL(promURL); err != nil {
160142
return nil, err
161143
}

0 commit comments

Comments
 (0)