From 9064081c2f7c427b06acb692e923bd9b678e8760 Mon Sep 17 00:00:00 2001 From: lorenzg Date: Thu, 29 Oct 2026 09:00:01 +0100 Subject: [PATCH 1/3] fix performance data bug in check_multi --- Changes | 1 + pkg/snclient/check_multi.go | 10 ++- pkg/snclient/check_multi_test.go | 106 +++++++++++++++++++++++++++++++ 3 files changed, 115 insertions(+), 2 deletions(-) diff --git a/Changes b/Changes index 298910d0..38f77ac4 100644 --- a/Changes +++ b/Changes @@ -3,6 +3,7 @@ This file documents the revision history for the SNClient agent. next: - update golang build chain to 1.27.3 - support multiline external commands and check_multi config + - check_multi: fix aggregating child performance data with the command tag prefix - check_eventlog: translate event type and source filter conditions into the WMI query - check_uptime: fix display glitch when uptime is exactly 3 weeks - add ppc64le rpm/deb builds diff --git a/pkg/snclient/check_multi.go b/pkg/snclient/check_multi.go index e1e9887e..414d7ee0 100644 --- a/pkg/snclient/check_multi.go +++ b/pkg/snclient/check_multi.go @@ -609,14 +609,20 @@ func (l *CheckMulti) runChildCheck(ctx context.Context, snc *Agent, chk multiChi cmdName := tokens[0] cmdArgs := tokens[1:] - _, isKnown := snc.getCheck(cmdName, false) + childCheck, isKnown := snc.getCheck(cmdName, false) if chk.isInline && !isKnown { return nil, fmt.Errorf("unknown check command: %s (inline checks only support existing check commands)", cmdName) } if isKnown { - return snc.RunCheckWithContext(ctx, cmdName, cmdArgs, 0, nil, false), nil + res := snc.RunCheckWithContext(ctx, cmdName, cmdArgs, 0, nil, false) + // Aliases already apply their own performance data settings. + if _, isAlias := childCheck.Handler().(*CheckAlias); !isAlias { + res.ParsePerformanceDataFromOutput() + } + + return res, nil } timeout := l.externalScriptTimeout(snc) diff --git a/pkg/snclient/check_multi_test.go b/pkg/snclient/check_multi_test.go index 5f2ec623..52c19798 100644 --- a/pkg/snclient/check_multi_test.go +++ b/pkg/snclient/check_multi_test.go @@ -2,15 +2,121 @@ package snclient import ( "fmt" + "net" "os" "path/filepath" "runtime" + "strings" "testing" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) +func TestCheckMultiTCPPerformanceData(t *testing.T) { + listener, err := net.Listen("tcp", "127.0.0.1:0") + require.NoError(t, err) + defer listener.Close() + _, port, err := net.SplitHostPort(listener.Addr().String()) + require.NoError(t, err) + + snc := StartTestAgent(t, "[/modules]\nCheckBuiltinPlugins = enabled\n") + defer StopTestAgent(t, snc) + + res := snc.RunCheck("check_multi", []string{ + "command[P01]=check_tcp -H 127.0.0.1 -p " + port + " -t 2", + "command[P02]=check_dummy 2 'TCP CRITICAL - connection failed'", + "crit=ok_count eq 0", + }) + assert.Equal(t, CheckExitOK, res.State) + assert.Contains(t, res.Output, "2 plugins checked: 1 ok, 0 warning, 1 critical, 0 unknown") + require.Len(t, res.Metrics, 8) + assert.Equal(t, "P01::time", res.Metrics[0].Name) + assert.Equal(t, "s", res.Metrics[0].Unit) + assert.True(t, res.Metrics[0].SkipStateCheck) + assert.Equal(t, "time", res.Metrics[7].Name) + firstLine, _, _ := strings.Cut(string(res.BuildPluginOutput()), "\n") + assert.Contains(t, firstLine, "'P01::time'=") + assert.Contains(t, res.Details, "[P01] TCP OK -") + assert.NotContains(t, res.Details, "|") +} + +func TestCheckMultiPerformanceData(t *testing.T) { + config := ` +[/modules] +CheckExternalScripts = enabled + +[/settings/external scripts/alias] +parsed = check_dummy 0 'OK | time=0.003234s;1;2;0;10' + +[/settings/external scripts/alias/ignored] +command = check_dummy 0 'OK | time=0.003234s;1;2;0;10' +ignore perfdata = yes + +[/settings/check/multi/inner] +command[leaf] = check_dummy 0 'OK | time=0.003234s;1;2;0;10' +` + snc := StartTestAgent(t, config) + defer StopTestAgent(t, snc) + + for _, testCase := range []struct { + name string + args []string + metric string + details string + metricsCount int + }{ + { + name: "raw", + args: []string{"command[child]=check_dummy 0 'OK | time=0.003234s;1;2;0;10'"}, + metric: "'child::time'=0.003234s;1;2;0;10", + details: "[child] OK", + metricsCount: 8, + }, + { + name: "already parsed", + args: []string{"command[child]=parsed"}, + metric: "'child::time'=0.003234s;1;2;0;10", + details: "[child] OK", + metricsCount: 8, + }, + { + name: "ignored", + args: []string{"command[child]=ignored"}, + details: "[child] OK | time=0.003234s;1;2;0;10", + metricsCount: 7, + }, + { + name: "filtered", + args: []string{ + "command[visible]=check_dummy 0 OK", + "command[hidden]=check_dummy 0 'OK | time=0.003234s;1;2;0;10'", + "filter=name eq 'visible'", + }, + details: "[visible] OK", + metricsCount: 7, + }, + { + name: "nested", + args: []string{"command[outer]=check_multi config=inner"}, + metric: "'outer::leaf::time'=0.003234s;1;2;0;10", + details: "[outer] OK - 1 plugins checked, 1 ok\n[leaf] OK", + metricsCount: 15, + }, + } { + t.Run(testCase.name, func(t *testing.T) { + res := snc.RunCheck("check_multi", testCase.args) + assert.Equal(t, CheckExitOK, res.State) + assert.Len(t, res.Metrics, testCase.metricsCount) + assert.Equal(t, testCase.details, res.Details) + if testCase.metric != "" { + firstLine, _, _ := strings.Cut(string(res.BuildPluginOutput()), "\n") + assert.Contains(t, firstLine, testCase.metric) + } + }) + } +} + func TestCheckMultiLineContinuation(t *testing.T) { config := ` [/settings/check/multi/multiline] From d3735fc0bf0e56eab1566c04609f2d23dd20d07a Mon Sep 17 00:00:00 2001 From: lorenzg Date: Thu, 29 Oct 2026 11:14:05 +0100 Subject: [PATCH 2/3] add vars to check_multi config --- docs/checks/commands/check_multi.md | 25 ++++++-- pkg/snclient/check_multi.go | 91 +++++++++++++++++++++++++---- pkg/snclient/check_multi_test.go | 73 +++++++++++++++++++++++ pkg/snclient/checkdata.go | 19 +++++- 4 files changed, 191 insertions(+), 17 deletions(-) diff --git a/docs/checks/commands/check_multi.md b/docs/checks/commands/check_multi.md index 22f1336c..7dddab3e 100644 --- a/docs/checks/commands/check_multi.md +++ b/docs/checks/commands/check_multi.md @@ -14,11 +14,25 @@ When using the inline mode, you can only use available commands (run check You can also define custom check sections in the config file, for example: [/settings/check/multi/mycheck] - command[alias1] = check_process process=123 + var[PROCESS] = 123 + command[alias1] = check_process process=$PROCESS$ command[alias2] = check_process process=345 This can be executed with check_multi "config=mycheck". +Use check_multi "config=mycheck" "var[PROCESS]=456" to override a variable's default value. +Variable names are case-sensitive. Every occurrence of a declared variable is replaced; undeclared placeholders stay unchanged. +Shell metacharacters and control characters are forbidden in values. Quote placeholders to preserve spaces. + +Variables without defaults must be declared as required (comma-separated): + + [/settings/check/multi/required] + required vars = MESSAGE + command[message] = check_dummy 0 '$MESSAGE$' + +Run this with check_multi "config=required" "var[MESSAGE]=Service is healthy". +A required variable must have a default or be explicitly supplied; an empty value counts as supplied. + It's also possible to use custom scripts in the config section, for example: [/settings/check/multi/myscript] @@ -54,10 +68,11 @@ This can be executed with check_multi "config=myscript". ## Check Specific Arguments -| Argument | Description | -| -------- | ------------------------------------------------------------------------- | -| command | Check command to execute with mandatory unique tag, e.g. command[tag]=... | -| config | Config section name under [/settings/check/multi/< section >] to execute | +| Argument | Description | +| -------- | ----------------------------------------------------------------------------------- | +| command | Check command to execute with mandatory unique tag, e.g. command[tag]=... | +| config | Config section name under [/settings/check/multi/< section >] to execute | +| var | Config variable override, e.g. var[KEY]=VALUE, replacing \$KEY\$ in config commands | ## Attributes diff --git a/pkg/snclient/check_multi.go b/pkg/snclient/check_multi.go index 414d7ee0..cc712a08 100644 --- a/pkg/snclient/check_multi.go +++ b/pkg/snclient/check_multi.go @@ -37,8 +37,9 @@ type checkMultiCounter struct { } type CheckMulti struct { - commands TaggedCommandList - config string + commands TaggedCommandList + config string + variables map[string]string } var checkMultiAttributes = []CheckAttribute{ @@ -59,14 +60,12 @@ var checkMultiAttributes = []CheckAttribute{ func NewCheckMulti() CheckHandler { return &CheckMulti{ - commands: make(TaggedCommandList, 0), + commands: make(TaggedCommandList, 0), + variables: make(map[string]string), } } -func (l *CheckMulti) Build() *CheckData { - return &CheckData{ - name: "check_multi", - description: `Runs multiple checks and aggregates their status, output and performance data. +const checkMultiDescription = `Runs multiple checks and aggregates their status, output and performance data. By default CheckMulti is enabled, but you can disable it in the [/modules] section of the snclient_local.ini. You can also set max checks in the [/settings/check/multi] section of the snclient_local.ini, which limits @@ -77,11 +76,25 @@ When using the inline mode, you can only use available commands (run check You can also define custom check sections in the config file, for example: [/settings/check/multi/mycheck] - command[alias1] = check_process process=123 + var[PROCESS] = 123 + command[alias1] = check_process process=$PROCESS$ command[alias2] = check_process process=345 This can be executed with check_multi "config=mycheck". +Use check_multi "config=mycheck" "var[PROCESS]=456" to override a variable's default value. +Variable names are case-sensitive. Every occurrence of a declared variable is replaced; undeclared placeholders stay unchanged. +Shell metacharacters and control characters are forbidden in values. Quote placeholders to preserve spaces. + +Variables without defaults must be declared as required (comma-separated): + + [/settings/check/multi/required] + required vars = MESSAGE + command[message] = check_dummy 0 '$MESSAGE$' + +Run this with check_multi "config=required" "var[MESSAGE]=Service is healthy". +A required variable must have a default or be explicitly supplied; an empty value counts as supplied. + It's also possible to use custom scripts in the config section, for example: [/settings/check/multi/myscript] @@ -89,7 +102,12 @@ It's also possible to use custom scripts in the config section, for example: command[alias2] = /path/to/plugin2 command[alias3] = /path/to/plugin3 -This can be executed with check_multi "config=myscript".`, +This can be executed with check_multi "config=myscript".` + +func (l *CheckMulti) Build() *CheckData { + return &CheckData{ + name: "check_multi", + description: checkMultiDescription, implemented: ALL, result: &CheckResult{ State: CheckExitOK, @@ -97,6 +115,7 @@ This can be executed with check_multi "config=myscript".`, args: map[string]CheckArgument{ "command": {value: &l.commands, description: "Check command to execute with mandatory unique tag, e.g. command[tag]=..."}, "config": {value: &l.config, description: "Config section name under [/settings/check/multi/< section >] to execute"}, + "var": {value: &l.variables, description: "Config variable override, e.g. var[KEY]=VALUE, replacing $KEY$ in config commands"}, }, conditionAlias: map[string]map[string]string{ "warning_count": {"warn_count": "warning_count"}, @@ -317,11 +336,23 @@ func (l *CheckMulti) buildConfigChecks(snc *Agent) ([]multiChildCheck, error) { } childChecks := make([]multiChildCheck, 0, len(sec.keys)) + variables, err := l.configVariables(sec) + if err != nil { + return nil, err + } + replacements := make([]string, 0, 2*len(variables)) + for key, value := range variables { + replacements = append(replacements, "$"+key+"$", value) + } + replacer := strings.NewReplacer(replacements...) for _, key := range sec.keys { + if key == "required vars" || (strings.HasPrefix(key, "var[") && strings.HasSuffix(key, "]")) { + continue + } rawVal := sec.data[key] if !strings.HasPrefix(key, "command[") || !strings.HasSuffix(key, "]") { - return nil, fmt.Errorf("invalid check_multi config entry: %s (must be in format command[tag]=)", key) + return nil, fmt.Errorf("invalid check_multi config entry: %s (expected command[tag], var[KEY], or required vars)", key) } tag := strings.TrimSpace(strings.TrimSuffix(strings.TrimPrefix(key, "command["), "]")) if strings.ContainsAny(tag, DefaultNastyCharacters+"=") { @@ -335,7 +366,7 @@ func (l *CheckMulti) buildConfigChecks(snc *Agent) ([]multiChildCheck, error) { } childChecks = append(childChecks, multiChildCheck{ tag: tag, - cmdStr: rawVal, + cmdStr: replacer.Replace(rawVal), isInline: false, }) } @@ -343,6 +374,44 @@ func (l *CheckMulti) buildConfigChecks(snc *Agent) ([]multiChildCheck, error) { return childChecks, nil } +func (l *CheckMulti) configVariables(sec *ConfigSection) (map[string]string, error) { + variables := make(map[string]string) + for _, key := range sec.keys { + if strings.HasPrefix(key, "var[") && strings.HasSuffix(key, "]") { + if err := setTaggedVariable(variables, key[4:len(key)-1], sec.data[key]); err != nil { + return nil, err + } + } + } + required, _ := sec.GetStringList("required vars") + for _, name := range required { + if _, ok := variables[name]; ok { + continue + } + value, ok := l.variables[name] + if !ok { + return nil, fmt.Errorf("missing required variable: %s", name) + } + if err := setTaggedVariable(variables, name, value); err != nil { + return nil, err + } + } + for name, value := range l.variables { + if _, ok := variables[name]; !ok { + continue + } + variables[name] = value + } + // Only declared values enter command strings, including on Windows. + for name, value := range variables { + if containsNastyCharacters(value, DefaultNastyCharacters+"%!^()*?~#") || containsControlCharacters(value) { + return nil, fmt.Errorf("variable %s contains illegal characters", name) + } + } + + return variables, nil +} + type childRecord struct { tag string command string diff --git a/pkg/snclient/check_multi_test.go b/pkg/snclient/check_multi_test.go index 52c19798..ab51cf3f 100644 --- a/pkg/snclient/check_multi_test.go +++ b/pkg/snclient/check_multi_test.go @@ -706,6 +706,79 @@ command[next] = check_multi config=depth6 assert.Contains(t, res.BuildOutputString(), "UNKNOWN - recursion limit exceeded for check_multi") } +func TestCheckMultiConfigVariables(t *testing.T) { + snc := StartTestAgent(t, ` +[/settings/check/multi/defaults] +var[abc] = default value +command[test] = check_dummy 0 '$abc$ / $abc$' + +[/settings/check/multi/required] +required vars = abc, host +var[host] = localhost +command[test] = check_dummy 0 '$abc$@$host$' + +[/settings/check/multi/literal] +command[test] = check_dummy 0 '$VALUE$' +`) + defer StopTestAgent(t, snc) + + tests := []struct { + name string + args []string + state int64 + output string + }{ + {"default", []string{"config=defaults"}, CheckExitOK, "[test] default value / default value"}, + {"override", []string{"config=defaults", "var[abc]=new value"}, CheckExitOK, "[test] new value / new value"}, + {"required missing", []string{"config=required"}, CheckExitUnknown, "missing required variable: abc"}, + {"required supplied", []string{"config=required", "var[abc]=value"}, CheckExitOK, "[test] value@localhost"}, + {"undeclared", []string{"config=literal"}, CheckExitOK, "[test] $VALUE$"}, + {"undeclared override", []string{"config=literal", "var[VALUE]=$(ignored)"}, CheckExitOK, "[test] $VALUE$"}, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + res := snc.RunCheck("check_multi", test.args) + assert.Equal(t, test.state, res.State) + assert.Contains(t, res.BuildOutputString(), test.output) + if test.state == CheckExitUnknown { + assert.Empty(t, res.Details) + } + }) + } +} + +func TestCheckMultiRejectsUnsafeVariableValues(t *testing.T) { + snc := StartTestAgent(t, ` +[/settings/check/multi/injection] +var[VALUE] = safe default +command[external] = /does/not/exist '$VALUE$' +`) + defer StopTestAgent(t, snc) + + values := []string{ + "safe; echo INJECTED", "$(echo INJECTED)", "`echo INJECTED`", + "safe & echo INJECTED", "safe | echo INJECTED", "safe > file", "safe < file", + "safe' 'extra", `safe" "extra`, `safe\ extra`, + "%COMSPEC%", "!COMSPEC!", "safe^&echo INJECTED", "(echo INJECTED)", + "$OTHER$", "*", "?", "~", "#", "[abc]", "{a,b}", + "safe\nextra", "safe\rextra", "safe\textra", "safe\x00extra", "safe\x1bextra", + } + for _, value := range values { + t.Run(fmt.Sprintf("%q", value), func(t *testing.T) { + res := snc.RunCheck("check_multi", []string{"config=injection", "var[VALUE]=" + value}) + assert.Equal(t, CheckExitUnknown, res.State) + assert.Equal(t, "UNKNOWN - variable VALUE contains illegal characters", res.Output) + assert.Empty(t, res.Details) + }) + } + if runtime.GOOS != "windows" { + snc.config.Section("/settings/check/multi/injection").Set("command[external]", "/bin/echo '$VALUE$'") + res := snc.RunCheck("check_multi", []string{"config=injection", "var[VALUE]=safe text 123 /path/file -arg=value@example.com:42,+"}) + assert.Equal(t, CheckExitOK, res.State) + assert.Equal(t, "[external] safe text 123 /path/file -arg=value@example.com:42,+", res.Details) + } +} + func TestCheckMultiConfigSection(t *testing.T) { // Create temporary scripts to test external scripts in config tmpDir := t.TempDir() diff --git a/pkg/snclient/checkdata.go b/pkg/snclient/checkdata.go index 8430c29f..22144f20 100644 --- a/pkg/snclient/checkdata.go +++ b/pkg/snclient/checkdata.go @@ -1113,6 +1113,19 @@ func (cd *CheckData) parseTaggedCommand(argRef *TaggedCommandList, tag, argValue return nil } +func setTaggedVariable(variables map[string]string, tag, value string) error { + tag = strings.TrimSpace(tag) + if tag == "" { + return fmt.Errorf("variable argument requires a name, e.g. var[KEY]=VALUE") + } + if strings.ContainsAny(tag, DefaultNastyCharacters+"=$[]") { + return fmt.Errorf("variable name contains invalid characters: %s", tag) + } + variables[tag] = value + + return nil +} + // parseAnyArg parses args into the args map with custom arguments func (cd *CheckData) parseAnyArg(argExpr, keyword, argValue string) (bool, error) { lookupKey := keyword @@ -1132,7 +1145,9 @@ func (cd *CheckData) parseAnyArg(argExpr, keyword, argValue string) (bool, error } } if hasTag { - if _, tagged := arg.value.(*TaggedCommandList); !tagged { + switch arg.value.(type) { + case *TaggedCommandList, *map[string]string: + default: return false, fmt.Errorf("argument %s does not support tags", lookupKey) } } @@ -1151,6 +1166,8 @@ func (cd *CheckData) parseArgValue(argExpr, keyword, argValue, tag string, arg * switch argRef := arg.value.(type) { case *TaggedCommandList: return cd.parseTaggedCommand(argRef, tag, argValue) + case *map[string]string: + return setTaggedVariable(*argRef, tag, argValue) case *[]string: if _, ok := cd.hasArgsSupplied[keyword]; !ok { // first time this arg occurs, empty default lists From ec1c93fd0a2cad323e0d9193c84590cb2feee711 Mon Sep 17 00:00:00 2001 From: lorenzg Date: Thu, 29 Oct 2026 11:23:57 +0100 Subject: [PATCH 3/3] update Changelog --- Changes | 1 + 1 file changed, 1 insertion(+) diff --git a/Changes b/Changes index 38f77ac4..baa0e18b 100644 --- a/Changes +++ b/Changes @@ -4,6 +4,7 @@ next: - update golang build chain to 1.27.3 - support multiline external commands and check_multi config - check_multi: fix aggregating child performance data with the command tag prefix + - check_multi: support config variables with defaults and required values - check_eventlog: translate event type and source filter conditions into the WMI query - check_uptime: fix display glitch when uptime is exactly 3 weeks - add ppc64le rpm/deb builds