app/vmselect/searchutil: prioritize URL query params over form values

When a request contains both URL path query params and POST form values
for extra_label and extra_filters[], URL query params now take
precedence. This resolves the conflict between the two sources and
simplifies security enforcement for extra_label/extra_filters policies
via vmauth or any other http proxy.

Fixes https://github.com/VictoriaMetrics/VictoriaMetrics/issues/10908
This commit is contained in:
Nikolay
2026-05-08 10:11:58 +02:00
committed by GitHub
parent 64f6c7e300
commit 87e59a4bbf
3 changed files with 34 additions and 3 deletions

View File

@@ -132,9 +132,20 @@ func (d *Deadline) String() string {
//
// {env="prod",team="devops",t1="v1",t2="v2"}
// {env=~"dev|staging",team!="devops",t1="v1",t2="v2"}
//
// Query args from URL path have precedence over post form args.
func GetExtraTagFilters(r *http.Request) ([][]storage.TagFilter, error) {
var tagFilters []storage.TagFilter
for _, match := range r.Form["extra_label"] {
urlQueryValues := r.URL.Query()
getRequestParam := func(key string) []string {
// query request param must always take precedence over form values
// in order to simplify security enforcement policy for extra_label and extra_filters
if uv, ok := urlQueryValues[key]; ok {
return uv
}
return r.Form[key]
}
for _, match := range getRequestParam("extra_label") {
tmp := strings.SplitN(match, "=", 2)
if len(tmp) != 2 {
return nil, fmt.Errorf("`extra_label` query arg must have the format `name=value`; got %q", match)
@@ -148,8 +159,8 @@ func GetExtraTagFilters(r *http.Request) ([][]storage.TagFilter, error) {
Value: []byte(tmp[1]),
})
}
extraFilters := append([]string{}, r.Form["extra_filters"]...)
extraFilters = append(extraFilters, r.Form["extra_filters[]"]...)
extraFilters := append([]string{}, getRequestParam("extra_filters")...)
extraFilters = append(extraFilters, getRequestParam("extra_filters[]")...)
if len(extraFilters) == 0 {
if len(tagFilters) == 0 {
return nil, nil

View File

@@ -20,6 +20,7 @@ func TestGetExtraTagFilters(t *testing.T) {
}
return &http.Request{
Form: q,
URL: &url.URL{RawQuery: q.Encode()},
}
}
f := func(t *testing.T, r *http.Request, want []string, wantErr bool) {
@@ -79,6 +80,24 @@ func TestGetExtraTagFilters(t *testing.T) {
nil,
false,
)
formValues, err := url.ParseQuery(`extra_label=env=prod&extra_label=job=vmsingle&extra_label=tenant=prod&extra_filters[]={foo="bar"}&extra_filters[]={tenant="prod"}`)
if err != nil {
t.Fatalf("BUG: cannot parse query: %s", err)
}
urlValues, err := url.ParseQuery(`extra_label=job=vmagent&extra_label=env=dev&extra_filters[]={tenant="dev"}`)
if err != nil {
t.Fatalf("BUG: cannot parse query: %s", err)
}
httpReqWithBothFormAndURLParams := &http.Request{
Form: formValues,
URL: &url.URL{
RawQuery: urlValues.Encode(),
},
}
f(t, httpReqWithBothFormAndURLParams,
[]string{`{tenant="dev",job="vmagent",env="dev"}`},
false)
}
func TestParseMetricSelectorSuccess(t *testing.T) {

View File

@@ -41,6 +41,7 @@ See also [LTS releases](https://docs.victoriametrics.com/victoriametrics/lts-rel
* BUGFIX: [vmauth](https://docs.victoriametrics.com/victoriametrics/vmauth/): now correctly respects disabling retries via `-maxRequestBodySizeToRetry=0`. Previously, disabling retries required setting both `-maxRequestBodySizeToRetry` and `-requestBufferSize` to `0`. See [#10857](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/10857). Thanks to @andriibeee for the contribution.
* BUGFIX: [vmbackupmanager](https://docs.victoriametrics.com/victoriametrics/vmbackupmanager/): explicitly set the MD5 checksum for `DeleteObjects` requests. Starting with [aws-sdk-go-v2/service/s3 v1.73.0](https://github.com/aws/aws-sdk-go-v2/blob/release-2025-01-15/service/s3/CHANGELOG.md#v1730-2025-01-15), the SDK switched the checksum algorithm from MD5 to CRC32, which [can break compatibility](https://github.com/aws/aws-sdk-go-v2/discussions/2960) with some third-party S3 implementations, including [Dell ECS](https://www.dell.com/en-us/lp/dt/elastic-cloud-storage). See [#10907](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/10907).
* BUGFIX: [vmauth](https://docs.victoriametrics.com/victoriametrics/vmauth/): attempt to route requests to the first backend when all backends are marked as broken. Previously, `vmauth` returned an error immediately without attempting any backend. This change may improve success rate in rare edge cases. See [#10837](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/10837).
* BUGFIX: [vmsingle](https://docs.victoriametrics.com/victoriametrics/single-server-victoriametrics/): and `vmselect` in [VictoriaMetrics cluster](https://docs.victoriametrics.com/victoriametrics/cluster-victoriametrics/): give priority to `extra_label` and `extra_filters[]` params defined in URL query string over those defined in request body form, to properly respect constraints imposed by `vmauth`. See [#10908](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/10908).
* BUGFIX: [vmsingle](https://docs.victoriametrics.com/victoriametrics/single-server-victoriametrics/) and `vmselect` in [VictoriaMetrics cluster](https://docs.victoriametrics.com/victoriametrics/cluster-victoriametrics/): set CORS headers on `/api/v1/export`, `/api/v1/export/csv` and `/api/v1/export/native`. See [#10899](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/10899). Thanks to @andriibeee for the contribution.
## [v1.142.0](https://github.com/VictoriaMetrics/VictoriaMetrics/releases/tag/v1.142.0)