From 6d6c606395ec09fd29c827db09b8fd2276ace8e5 Mon Sep 17 00:00:00 2001 From: immanuwell Date: Tue, 30 Jun 2026 20:18:06 +0400 Subject: [PATCH] fix: validate dynamic filter regexes during config loading Signed-off-by: immanuwell --- collector/collector.go | 14 +++++++------- collector/collector_test.go | 18 +++++++++++++++++- config/config.go | 32 +++++++++++++++++++++++++++++--- config/config_test.go | 21 +++++++++++++++++++++ 4 files changed, 74 insertions(+), 11 deletions(-) diff --git a/collector/collector.go b/collector/collector.go index cac3de7f..b4e70cdb 100644 --- a/collector/collector.go +++ b/collector/collector.go @@ -19,7 +19,6 @@ import ( "encoding/hex" "fmt" "log/slog" - "regexp" "strconv" "strings" "sync" @@ -101,7 +100,8 @@ func ScrapeTarget(snmp scraper.SNMPScraper, target string, auth *config.Auth, mo allowedIndicesByTarget := map[string][]string{} var filteredTargets []string - for _, filter := range module.Filters { + for i := range module.Filters { + filter := &module.Filters[i] allowedList := []string{} pdus, err := snmp.WalkAll(filter.Oid) // Do not try to filter anything if we had errors. @@ -113,7 +113,7 @@ func ScrapeTarget(snmp scraper.SNMPScraper, target string, auth *config.Auth, mo allowedList = filterAllowedIndices(logger, filter, pdus, allowedList, metrics) // Update config to get only index and not walk them. - newWalk = updateWalkConfig(newWalk, filter, logger) + newWalk = updateWalkConfig(newWalk, *filter, logger) for _, targetOid := range filter.Targets { if existing, ok := allowedIndicesByTarget[targetOid]; ok { @@ -195,15 +195,15 @@ func intersectIndices(a, b []string) []string { return result } -func filterAllowedIndices(logger *slog.Logger, filter config.DynamicFilter, pdus []gosnmp.SnmpPDU, allowedList []string, metrics Metrics) []string { +func filterAllowedIndices(logger *slog.Logger, filter *config.DynamicFilter, pdus []gosnmp.SnmpPDU, allowedList []string, metrics Metrics) []string { logger.Debug("Evaluating rule for oid", "oid", filter.Oid) for _, pdu := range pdus { found := false - for _, val := range filter.Values { + for i, re := range filter.Regexps() { snmpval := pduValueAsString(&pdu, "DisplayString", "", metrics) - logger.Debug("evaluating filters", "config value", val, "snmp value", snmpval) + logger.Debug("evaluating filters", "config value", filter.Values[i], "snmp value", snmpval) - if regexp.MustCompile(val).MatchString(snmpval) { + if re.MatchString(snmpval) { found = true break } diff --git a/collector/collector_test.go b/collector/collector_test.go index 9b92ea7f..e9455f65 100644 --- a/collector/collector_test.go +++ b/collector/collector_test.go @@ -29,6 +29,20 @@ import ( "github.com/prometheus/snmp_exporter/scraper" ) +func mustCompileDynamicFilter(t *testing.T, filter *config.DynamicFilter) { + t.Helper() + if err := filter.CompileRegexps(); err != nil { + t.Fatalf("compiling dynamic filter regexps: %v", err) + } +} + +func mustCompileDynamicFilters(t *testing.T, filters []config.DynamicFilter) { + t.Helper() + for i := range filters { + mustCompileDynamicFilter(t, &filters[i]) + } +} + func TestPduToSample(t *testing.T) { cases := []struct { pdu *gosnmp.SnmpPDU @@ -1344,7 +1358,8 @@ func TestFilterAllowedIndices(t *testing.T) { }, } for _, c := range cases { - got := filterAllowedIndices(promslog.NewNopLogger(), c.filter, pdus, c.allowedList, Metrics{}) + mustCompileDynamicFilter(t, &c.filter) + got := filterAllowedIndices(promslog.NewNopLogger(), &c.filter, pdus, c.allowedList, Metrics{}) if !reflect.DeepEqual(got, c.result) { t.Errorf("filterAllowedIndices(%v): got %v, want %v", c.filter, got, c.result) } @@ -1636,6 +1651,7 @@ func TestScrapeTarget(t *testing.T) { for _, c := range cases { tt := c t.Run(tt.name, func(t *testing.T) { + mustCompileDynamicFilters(t, tt.module.Filters) mock := scraper.NewMockSNMPScraper(tt.getResponse, tt.walkResponses) results, err := ScrapeTarget(mock, "someTarget", auth, tt.module, promslog.NewNopLogger(), Metrics{}) if err != nil { diff --git a/config/config.go b/config/config.go index 5e89b995..c80ded16 100644 --- a/config/config.go +++ b/config/config.go @@ -230,9 +230,35 @@ type StaticFilter struct { Indices []string `yaml:"indices,omitempty"` } type DynamicFilter struct { - Oid string `yaml:"oid"` - Targets []string `yaml:"targets,omitempty"` - Values []string `yaml:"values,omitempty"` + Oid string `yaml:"oid"` + Targets []string `yaml:"targets,omitempty"` + Values []string `yaml:"values,omitempty"` + regexps []*regexp.Regexp `yaml:"-"` +} + +func (c *DynamicFilter) CompileRegexps() error { + regexps := make([]*regexp.Regexp, 0, len(c.Values)) + for _, value := range c.Values { + re, err := regexp.Compile(value) + if err != nil { + return fmt.Errorf("invalid dynamic filter value %q: %w", value, err) + } + regexps = append(regexps, re) + } + c.regexps = regexps + return nil +} + +func (c *DynamicFilter) Regexps() []*regexp.Regexp { + return c.regexps +} + +func (c *DynamicFilter) UnmarshalYAML(unmarshal func(any) error) error { + type plain DynamicFilter + if err := unmarshal((*plain)(c)); err != nil { + return err + } + return c.CompileRegexps() } type Metric struct { diff --git a/config/config_test.go b/config/config_test.go index 5e5ae649..fcd90248 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -14,6 +14,7 @@ package config import ( + "strings" "testing" "go.yaml.in/yaml/v2" @@ -56,3 +57,23 @@ modules: t.Error("BUG: module1 and module2 share the same Retries pointer!") } } + +func TestDynamicFilterRejectsInvalidRegex(t *testing.T) { + content := ` +modules: + module1: + walk: ["1.2.4"] + filters: + - oid: "1.2.3" + targets: ["1.2.4"] + values: ["("] +` + cfg := &Config{} + err := yaml.UnmarshalStrict([]byte(content), cfg) + if err == nil { + t.Fatal("expected invalid dynamic filter regex to be rejected") + } + if !strings.Contains(err.Error(), `invalid dynamic filter value "("`) { + t.Fatalf("unexpected error: %v", err) + } +}