From 91542cf34b5744f72ca999ce5740a13db270d02e Mon Sep 17 00:00:00 2001 From: Ahmet Oeztuerk Date: Tue, 14 Jul 2026 18:02:45 +0200 Subject: [PATCH 1/6] add two sanity functions to check: checkThresholdKeywordsAgainstAttributeNames -> this prints to log about keywords that are used in thresholds, but are not defined in check attributes. they are not fatal to the check, as thresholds can work on perfdata metric labels as well checkFilterKeywordsAgainstAttributeNames -> this does the same against filter now, afaik filters only work on attribute names, so using a wrong keyword means a user problem. this is fatal for the check call these two functions after parsing the arguments of a check, and before running it other fixes: switch to using ConditionList.GetListOfKeywords in CheckData.GetAllThresholdKeywords fix ConditionList.GetListOfKeywords adding empty keywords, group conditions have empty keywords --- pkg/snclient/checkdata.go | 91 ++++++++++++++++++++++++++-------- pkg/snclient/checkdata_test.go | 27 ++++++++++ pkg/snclient/condition.go | 21 +++++++- pkg/snclient/snclient.go | 10 ++++ 4 files changed, 127 insertions(+), 22 deletions(-) diff --git a/pkg/snclient/checkdata.go b/pkg/snclient/checkdata.go index f2303eef..656e63c4 100644 --- a/pkg/snclient/checkdata.go +++ b/pkg/snclient/checkdata.go @@ -858,6 +858,67 @@ func (cd *CheckData) preParseArgs(args []string) (sanitized []Argument, defaultW return sanitized, defaultWarning, defaultCritical, applyDefaultFilter, nil } +// Threshold keywords do not necessarily have to match an attribute name. +// They can be written to match perfdata metric names +// They can also be match to check details, but that is less common +// Therefore, this function does not return an error +func (cd *CheckData) checkThresholdKeywordsAgainstAttributeNames() { + attributeNames := cd.getAttributeNames() + + warningKeywords, err := cd.warnThreshold.GetListOfKeywords() + if err == nil && len(warningKeywords) > 0 { + warningKeywordsExtra := utils.SubtractSlice(warningKeywords, attributeNames) + if len(warningKeywordsExtra) > 0 { + log.Tracef("Warning condition uses keyword(s) not present in the attributes, run with --help to get a list of attributes, extra keywords: %v", warningKeywordsExtra) + } + } + + critKeywords, err := cd.critThreshold.GetListOfKeywords() + if err == nil && len(critKeywords) > 0 { + critKeywordsExtra := utils.SubtractSlice(critKeywords, attributeNames) + if len(critKeywordsExtra) > 0 { + log.Tracef("Crit condition uses keyword(s) not present in the attributes, run with --help to get a list of attributes, extra keywords: %v", critKeywordsExtra) + } + } + + okKeywords, err := cd.okThreshold.GetListOfKeywords() + if err == nil && len(okKeywords) > 0 { + okKeywordsExtra := utils.SubtractSlice(okKeywords, attributeNames) + if len(okKeywordsExtra) > 0 { + log.Tracef("Ok condition uses keyword(s) not present in the attributes, run with --help to get a list of attributes, extra keywords: %v", okKeywordsExtra) + } + } +} + +// Filter keywords should match attribute filters. +// They are used to filter entries added to the check. Entries use attributes. +// They are not compared with perfdata metric names. +// They are not compared with check details. +// Not matching an attribute name means the condition has no effect, and is likely wrong +func (cd *CheckData) checkFilterKeywordsAgainstAttributeNames() (err error) { + attributeNames := cd.getAttributeNames() + + filterKeywords, err := cd.filter.GetListOfKeywords() + if err == nil && len(filterKeywords) > 0 { + filterKeywordsExtra := utils.SubtractSlice(filterKeywords, attributeNames) + if len(filterKeywordsExtra) > 0 { + return fmt.Errorf("filter condition uses keyword(s) not present in the attributes: %s", strings.Join(filterKeywordsExtra, ", ")) + } + } + + return nil +} + +func (cd *CheckData) getAttributeNames() (attributeNames []string) { + attributeNames = make([]string, 0, len(cd.attributes)) + + for _, attribute := range cd.attributes { + attributeNames = append(attributeNames, attribute.name) + } + + return attributeNames +} + func (cd *CheckData) fetchNextArg(args, split []string, keyword string, idx, numArgs int) (argVal string, newIdx int, err error) { if len(split) == 2 { return split[1], idx, nil @@ -1122,15 +1183,18 @@ func (cd *CheckData) HasThreshold(name string) bool { // GetAllThresholdKeywords returns a list of all keywords used in warn/crit/ok thresholds. func (cd *CheckData) GetAllThresholdKeywords() []string { - keywords := []string{} + keywords := make([]string, 0, len(cd.warnThreshold)+len(cd.critThreshold)+len(cd.okThreshold)) - keywords = append(keywords, cd.getAllThresholdKeywords(cd.warnThreshold)...) - keywords = append(keywords, cd.getAllThresholdKeywords(cd.critThreshold)...) - keywords = append(keywords, cd.getAllThresholdKeywords(cd.okThreshold)...) + warnThresholdKeywords, _ := cd.warnThreshold.GetListOfKeywords() + critThresholdKeywords, _ := cd.critThreshold.GetListOfKeywords() + okThresholdKeywords, _ := cd.okThreshold.GetListOfKeywords() - // make list unique + keywords = append(keywords, warnThresholdKeywords...) + keywords = append(keywords, critThresholdKeywords...) + keywords = append(keywords, okThresholdKeywords...) + + utils.Deduplicate(keywords) slices.Sort(keywords) - keywords = slices.Compact(keywords) return keywords } @@ -1150,21 +1214,6 @@ func (cd *CheckData) hasThresholdCond(condList ConditionList, name string) bool return false } -// hasThresholdCond returns true is the given list of conditions uses the given name at least once. -func (cd *CheckData) getAllThresholdKeywords(condList ConditionList) []string { - keywords := []string{} - - for _, cond := range condList { - if len(cond.group) > 0 { - keywords = append(keywords, cd.getAllThresholdKeywords(cond.group)...) - } - - keywords = append(keywords, cond.keyword) - } - - return keywords -} - // SetDefaultThresholdUnit sets default unit for all threshold conditions matching // the name and not having a unit already func (cd *CheckData) SetDefaultThresholdUnit(defaultUnit string, names []string) { diff --git a/pkg/snclient/checkdata_test.go b/pkg/snclient/checkdata_test.go index 726faa46..0a540740 100644 --- a/pkg/snclient/checkdata_test.go +++ b/pkg/snclient/checkdata_test.go @@ -25,3 +25,30 @@ func TestAllRequiredMacros(t *testing.T) { macros = check.AllRequiredMacros() assert.Equal(t, []string{"blah", "column1", "column29", "test"}, macros) } + +func TestCheckingFilterKeywordsOnCheckAttributes(t *testing.T) { + check := &CheckData{ + name: "testcheck", + description: "This test check simply has couple arguments.", + attributes: []CheckAttribute{ + { + name: "attribute1", + description: "attribute1", + }, + { + name: "attribute2", + description: "attribute2", + }, + { + name: "attribute3", + description: "attribute3", + }, + }, + } + + _, err := check.parseArgs([]string{"filter='(attribute1 eq value1) and (attribute2 like value2) and (attribute3 != value3)'", "filter='attribute4 eq value4'"}) + require.NoError(t, err, "should not have a problem parsing arguments") + + err = check.checkFilterKeywordsAgainstAttributeNames() + require.Error(t, err, "should error due to filter using keyword 'attribute4', which is not an attribute of check") +} diff --git a/pkg/snclient/condition.go b/pkg/snclient/condition.go index 8fe92874..d7df3e43 100644 --- a/pkg/snclient/condition.go +++ b/pkg/snclient/condition.go @@ -992,7 +992,9 @@ func (c *Condition) TransformMultipleKeywords(srcKeywords []string, targetKeywor // recursively gets list of all keywords used in the condition func (c *Condition) GetListOfKeywords() (keywords []string, err error) { addKeywordToList := func(c *Condition) (err error) { - keywords = append(keywords, c.keyword) + if c.keyword != "" { + keywords = append(keywords, c.keyword) + } return nil } @@ -1532,3 +1534,20 @@ func (cl *ConditionList) performMatches(data map[string]string, returnOnFirstMat return correctConditions, falseConditions, conclusiveConditions, inconclusiveConditions } + +func (cl *ConditionList) GetListOfKeywords() (keywords []string, err error) { + keywords = make([]string, 0) + + for _, cond := range *cl { + condKeywords, err := cond.GetListOfKeywords() + if err != nil { + return nil, fmt.Errorf("error gathering keywords: %s", err.Error()) + } + + keywords = append(keywords, condKeywords...) + } + + keywords = utils.Deduplicate(keywords) + + return keywords, nil +} diff --git a/pkg/snclient/snclient.go b/pkg/snclient/snclient.go index 80eec0b4..1415ef65 100644 --- a/pkg/snclient/snclient.go +++ b/pkg/snclient/snclient.go @@ -824,6 +824,16 @@ func (snc *Agent) runCheck(ctx context.Context, name string, args []string, time if chk.showHelp > 0 { return snc.runHelp(ctx, chk, handler), chk } + + chk.checkThresholdKeywordsAgainstAttributeNames() + err = chk.checkFilterKeywordsAgainstAttributeNames() + if err != nil { + return &CheckResult{ + State: CheckExitUnknown, + Output: fmt.Sprintf("${status} - %s", err.Error()), + }, chk + } + if !skipAllowedCheck { err = snc.checkAllowed(name, chk, handler, args, ArgumentList(parsedArgs).RawList(), transportConf) if err != nil { From 669c01a33e078bc73e2f1e2cf97e3341a6be37bc Mon Sep 17 00:00:00 2001 From: Ahmet Oeztuerk Date: Tue, 14 Jul 2026 18:25:08 +0200 Subject: [PATCH 2/6] add 'temperature' to the attribute list of check_temperature it was already being added to entry, but was not written in the check definition. it uses a default filter 'temperature != 0 and temperature != 1' and tests were failing since 'temperature' is not in attribute list --- docs/checks/commands/check_temperature.md | 19 ++++++++++--------- pkg/snclient/check_temperature.go | 2 ++ 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/docs/checks/commands/check_temperature.md b/docs/checks/commands/check_temperature.md index 2157e86d..37338958 100644 --- a/docs/checks/commands/check_temperature.md +++ b/docs/checks/commands/check_temperature.md @@ -69,12 +69,13 @@ Naemon Config these can be used in filters and thresholds (along with the default attributes): -| Attribute | Description | -| --------- | ---------------------------------------------- | -| sensor | full name of this sensor, ex.: coretemp_core_0 | -| name | name of this sensor, ex.: coretemp | -| label | label for this sensor, ex.: core 0 | -| value | current temperature | -| crit | critical value supplied from sensor | -| max | max value supplied from sensor | -| min | min value supplied from sensor | +| Attribute | Description | +| ----------- | ------------------------------------------------------------------------------------------------------- | +| sensor | full name of this sensor, ex.: coretemp_core_0 | +| name | name of this sensor, ex.: coretemp | +| label | label for this sensor, ex.: core 0 | +| value | current temperature | +| crit | critical value supplied from sensor | +| max | max value supplied from sensor | +| min | min value supplied from sensor | +| temperature | raw temperature value. this can be a wrong/misleading value like 0 or 1 if sensor is disabled. use 'value' attribute instead. | diff --git a/pkg/snclient/check_temperature.go b/pkg/snclient/check_temperature.go index 12dbc285..00c80dd3 100644 --- a/pkg/snclient/check_temperature.go +++ b/pkg/snclient/check_temperature.go @@ -57,6 +57,8 @@ func (l *CheckTemperature) Build() *CheckData { {name: "crit", description: "critical value supplied from sensor"}, {name: "max", description: "max value supplied from sensor"}, {name: "min", description: "min value supplied from sensor"}, + {name: "temperature", description: "raw temperature value." + + " this can be a wrong/misleading value like 0 or 1 if sensor is disabled. use 'value' attribute instead."}, }, listSorted: []string{"label"}, exampleDefault: ` From a0f700b27c277132a2a2413554b143cd1bed0c5b Mon Sep 17 00:00:00 2001 From: Ahmet Oeztuerk Date: Tue, 14 Jul 2026 18:27:17 +0200 Subject: [PATCH 3/6] move checking of filter keywords afterwards of checkAllowed(), and only perform them if chk.argsPassthrough is false this means security checks are done first, then semantic checks. also fixes some of the failing tests --- pkg/snclient/snclient.go | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/pkg/snclient/snclient.go b/pkg/snclient/snclient.go index 1415ef65..3dd285d2 100644 --- a/pkg/snclient/snclient.go +++ b/pkg/snclient/snclient.go @@ -825,15 +825,6 @@ func (snc *Agent) runCheck(ctx context.Context, name string, args []string, time return snc.runHelp(ctx, chk, handler), chk } - chk.checkThresholdKeywordsAgainstAttributeNames() - err = chk.checkFilterKeywordsAgainstAttributeNames() - if err != nil { - return &CheckResult{ - State: CheckExitUnknown, - Output: fmt.Sprintf("${status} - %s", err.Error()), - }, chk - } - if !skipAllowedCheck { err = snc.checkAllowed(name, chk, handler, args, ArgumentList(parsedArgs).RawList(), transportConf) if err != nil { @@ -844,6 +835,17 @@ func (snc *Agent) runCheck(ctx context.Context, name string, args []string, time } } + if !chk.argsPassthrough { + chk.checkThresholdKeywordsAgainstAttributeNames() + err = chk.checkFilterKeywordsAgainstAttributeNames() + if err != nil { + return &CheckResult{ + State: CheckExitUnknown, + Output: fmt.Sprintf("${status} - %s", err.Error()), + }, chk + } + } + if timeoutOverride > 0 { chk.timeout = timeoutOverride } From a787dc2a85279852356ace8503e51dcedc6402ec Mon Sep 17 00:00:00 2001 From: Ahmet Oeztuerk Date: Tue, 14 Jul 2026 18:40:19 +0200 Subject: [PATCH 4/6] make error message in extra filter keywords same as extra threshold keywords --- pkg/snclient/checkdata.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/snclient/checkdata.go b/pkg/snclient/checkdata.go index 656e63c4..43db310b 100644 --- a/pkg/snclient/checkdata.go +++ b/pkg/snclient/checkdata.go @@ -902,7 +902,7 @@ func (cd *CheckData) checkFilterKeywordsAgainstAttributeNames() (err error) { if err == nil && len(filterKeywords) > 0 { filterKeywordsExtra := utils.SubtractSlice(filterKeywords, attributeNames) if len(filterKeywordsExtra) > 0 { - return fmt.Errorf("filter condition uses keyword(s) not present in the attributes: %s", strings.Join(filterKeywordsExtra, ", ")) + return fmt.Errorf("filter condition uses keyword(s) not present in the attributes, run with --help to get a list of attributes, extra keywords: %s", strings.Join(filterKeywordsExtra, ", ")) } } From e393aa332853295f74c6a0098f7af80fd136b392 Mon Sep 17 00:00:00 2001 From: Ahmet Oeztuerk Date: Wed, 15 Jul 2026 10:50:27 +0200 Subject: [PATCH 5/6] checks: add extraFilterArgs attribute, where not-fixed, dynamically named attributes can be defined using regex. use these in conjunction with fixed attribute names when checking for keywords in the condition string add extra filter args to check_logfile 'column*' and check_wmi '.*' --- pkg/snclient/check_logfile.go | 3 +++ pkg/snclient/check_wmi.go | 4 ++++ pkg/snclient/checkdata.go | 24 +++++++++++++++++++++--- 3 files changed, 28 insertions(+), 3 deletions(-) diff --git a/pkg/snclient/check_logfile.go b/pkg/snclient/check_logfile.go index 376278f4..79cb8b6b 100644 --- a/pkg/snclient/check_logfile.go +++ b/pkg/snclient/check_logfile.go @@ -85,6 +85,9 @@ func (c *CheckLogFile) Build() *CheckData { {name: "line", description: "Match the content of an entire line"}, {name: "columnN", description: "Match the content of the N-th column only if enough columns exists"}, }, + extraFilterAttributes: []*regexp.Regexp{ + regexp.MustCompile(`column.*`), + }, exampleDefault: ` Alert if there are errors in the snclient log file: diff --git a/pkg/snclient/check_wmi.go b/pkg/snclient/check_wmi.go index 80f007df..cca4628b 100644 --- a/pkg/snclient/check_wmi.go +++ b/pkg/snclient/check_wmi.go @@ -3,6 +3,7 @@ package snclient import ( "context" "fmt" + "regexp" "runtime" "strings" @@ -49,6 +50,9 @@ func (l *CheckWMI) Build() *CheckData { hasArgsFilter: true, // otherwise empty-syntax won't be applied topSyntax: "${list}", detailSyntax: "%(line)", + extraFilterAttributes: []*regexp.Regexp{ + regexp.MustCompile(`.*`), + }, exampleDefault: ` check_wmi "query=select DeviceID, FreeSpace FROM Win32_LogicalDisk WHERE DeviceID = 'C:'" C:, 27955118080 diff --git a/pkg/snclient/checkdata.go b/pkg/snclient/checkdata.go index 43db310b..101e514f 100644 --- a/pkg/snclient/checkdata.go +++ b/pkg/snclient/checkdata.go @@ -3,6 +3,7 @@ package snclient import ( "fmt" "math" + "regexp" "slices" "sort" "strconv" @@ -137,6 +138,7 @@ type CheckData struct { output OutputMode implemented Implemented attributes []CheckAttribute + extraFilterAttributes []*regexp.Regexp listSorted []string // sort result list by this keys exampleDefault string exampleArgs string @@ -897,15 +899,31 @@ func (cd *CheckData) checkThresholdKeywordsAgainstAttributeNames() { // Not matching an attribute name means the condition has no effect, and is likely wrong func (cd *CheckData) checkFilterKeywordsAgainstAttributeNames() (err error) { attributeNames := cd.getAttributeNames() + regexes := make([]*regexp.Regexp, 0, len(attributeNames)+len(cd.extraFilterAttributes)) + + for _, attributeName := range attributeNames { + regexes = append(regexes, regexp.MustCompile(regexp.QuoteMeta(attributeName))) + } + + regexes = append(regexes, cd.extraFilterAttributes...) filterKeywords, err := cd.filter.GetListOfKeywords() + + filterKeywordsUnmatched := []string{} if err == nil && len(filterKeywords) > 0 { - filterKeywordsExtra := utils.SubtractSlice(filterKeywords, attributeNames) - if len(filterKeywordsExtra) > 0 { - return fmt.Errorf("filter condition uses keyword(s) not present in the attributes, run with --help to get a list of attributes, extra keywords: %s", strings.Join(filterKeywordsExtra, ", ")) + for _, filterKeyword := range filterKeywords { + if !slices.ContainsFunc(regexes, func(r *regexp.Regexp) bool { return r.MatchString(filterKeyword) }) { + filterKeywordsUnmatched = append(filterKeywordsUnmatched, filterKeyword) + } } } + if len(filterKeywordsUnmatched) > 0 { + log.Warnf("Filter condition uses keyword(s) not present in the attribute, filter condition: '%s' , extra keywords: '%s' ", cd.filter.String(), strings.Join(filterKeywordsUnmatched, ", ")) + + return fmt.Errorf("filter condition uses unknown attribute '%s', run with --help to get a list of attributes", filterKeywordsUnmatched[0]) + } + return nil } From 2615aa204f9dd3706ef503d1bade41abe983721e Mon Sep 17 00:00:00 2001 From: Ahmet Oeztuerk Date: Wed, 15 Jul 2026 11:17:59 +0200 Subject: [PATCH 6/6] add new dynamic attribute names to be used for filter keyword checking check_drivesize: flags, letter, opts check_logfile: column (adjusted regex) check_memory: physical, swap, committed, virtual check_os_updates: old_version, repository, arch, prefix check_pdf: name --- pkg/snclient/check_drivesize.go | 6 ++++++ pkg/snclient/check_logfile.go | 2 +- pkg/snclient/check_memory.go | 7 +++++++ pkg/snclient/check_os_updates.go | 7 +++++++ pkg/snclient/check_pdh.go | 4 ++++ 5 files changed, 25 insertions(+), 1 deletion(-) diff --git a/pkg/snclient/check_drivesize.go b/pkg/snclient/check_drivesize.go index f1cc5bbe..aaeb3b3a 100644 --- a/pkg/snclient/check_drivesize.go +++ b/pkg/snclient/check_drivesize.go @@ -5,6 +5,7 @@ import ( "errors" "fmt" "maps" + "regexp" "runtime" "sort" "strconv" @@ -154,6 +155,11 @@ func (l *CheckDrivesize) Build() *CheckData { {name: "localised_remote_path", description: "Windows only: If the path is given as a remote path, and that remote path has an assigned logical drive," + " this is the replaced path under that logical drive."}, }, + extraFilterAttributes: []*regexp.Regexp{ + regexp.MustCompile(`^flags$`), + regexp.MustCompile(`^letter$`), + regexp.MustCompile(`^opts$`), + }, exampleDefault: l.getExample(), exampleArgs: `'warn=used_pct > 90' 'crit=used_pct > 95'`, } diff --git a/pkg/snclient/check_logfile.go b/pkg/snclient/check_logfile.go index 79cb8b6b..7d043d11 100644 --- a/pkg/snclient/check_logfile.go +++ b/pkg/snclient/check_logfile.go @@ -86,7 +86,7 @@ func (c *CheckLogFile) Build() *CheckData { {name: "columnN", description: "Match the content of the N-th column only if enough columns exists"}, }, extraFilterAttributes: []*regexp.Regexp{ - regexp.MustCompile(`column.*`), + regexp.MustCompile(`^column.*`), }, exampleDefault: ` Alert if there are errors in the snclient log file: diff --git a/pkg/snclient/check_memory.go b/pkg/snclient/check_memory.go index e48ffc7e..30a24439 100644 --- a/pkg/snclient/check_memory.go +++ b/pkg/snclient/check_memory.go @@ -3,6 +3,7 @@ package snclient import ( "context" "fmt" + "regexp" "runtime" "github.com/consol-monitoring/snclient/pkg/humanize" @@ -76,6 +77,12 @@ read more on windows virtual address space: {name: "size", description: "Total memory in human readable bytes (IEC)", unit: UByte}, {name: "size_bytes", description: "Total memory in bytes (IEC)", unit: UByte}, }, + extraFilterAttributes: []*regexp.Regexp{ + regexp.MustCompile(`^(?i)physical$`), + regexp.MustCompile(`^(?i)swap$`), + regexp.MustCompile(`^(?i)committed$`), + regexp.MustCompile(`^(?i)virtual$`), + }, exampleDefault: ` check_memory OK - physical = 6.98 GiB, committed = 719.32 MiB|... diff --git a/pkg/snclient/check_os_updates.go b/pkg/snclient/check_os_updates.go index 49243ce6..c2146020 100644 --- a/pkg/snclient/check_os_updates.go +++ b/pkg/snclient/check_os_updates.go @@ -4,6 +4,7 @@ import ( "cmp" "context" "fmt" + "regexp" "slices" "github.com/consol-monitoring/snclient/pkg/convert" @@ -49,6 +50,12 @@ func (l *CheckOSUpdates) Build() *CheckData { {name: "security", description: "is this a security update: 0 / 1"}, {name: "version", description: "version string of package"}, }, + extraFilterAttributes: []*regexp.Regexp{ + regexp.MustCompile(`^old_version$`), + regexp.MustCompile(`^repository$`), + regexp.MustCompile(`^arch$`), + regexp.MustCompile(`^prefix$`), + }, exampleDefault: ` check_os_updates OK - no updates available |... diff --git a/pkg/snclient/check_pdh.go b/pkg/snclient/check_pdh.go index c2818e51..f5ad662e 100644 --- a/pkg/snclient/check_pdh.go +++ b/pkg/snclient/check_pdh.go @@ -2,6 +2,7 @@ package snclient import ( "context" + "regexp" ) func init() { @@ -48,6 +49,9 @@ func (c *CheckPDH) Build() *CheckData { {name: "count ", description: "Number of items matching the filter. Common option for all checks."}, {name: "value ", description: "The counter value (either float or int)"}, }, + extraFilterAttributes: []*regexp.Regexp{ + regexp.MustCompile(`^name$`), + }, exampleDefault: ` check_pdh "counter=foo" "warn=value > 80" "crit=value > 90" Everything looks good