diff --git a/cmd/mcp/main.go b/cmd/mcp/main.go index 9d64ade5..192f8964 100644 --- a/cmd/mcp/main.go +++ b/cmd/mcp/main.go @@ -131,6 +131,8 @@ func run(ctx context.Context, _ *cli.Command) error { if err != nil { return fmt.Errorf("error creating API client - %w", err) } + // Distinguishes agent traffic from osctrl-cli's in osctrl-api's logs. + client.UserAgent = serviceName + "/" + buildVersion // Fail fast on a bad URL or a dead/rejected token. Without this the // server starts happily and every tool call fails one at a time, which // reads to the user as "the tools are broken" rather than "the token is diff --git a/pkg/alerts/matcher.go b/pkg/alerts/matcher.go index 2ad9ee59..e5f6218d 100644 --- a/pkg/alerts/matcher.go +++ b/pkg/alerts/matcher.go @@ -3,6 +3,7 @@ package alerts import ( "encoding/json" "regexp" + "slices" "strings" "github.com/jmpsec/osctrl/pkg/types" @@ -60,6 +61,7 @@ func (rs *RuleSet) MatchResultLogs(envID uint, environment, nodeUUID string, log entry := &logs[i] uuid := nodeUUID fields := resultFields(entry) + keys := sortedKeys(fields) lowered := make(map[string]string, len(fields)) for j := range rs.result { r := &rs.result[j] @@ -70,7 +72,7 @@ func (rs *RuleSet) MatchResultLogs(envID uint, environment, nodeUUID string, log if r.nodeScope != "" && r.nodeScope != uuid { continue } - if matched, detail := matchFields(r, fields, lowered); matched { + if matched, detail := matchFields(r, fields, keys, lowered); matched { hits = append(hits, Hit{ RuleID: r.id, RuleName: r.name, @@ -145,6 +147,7 @@ func (rs *RuleSet) MatchQueryResult(envID uint, environment, queryName string, r if !ok { return nil } + keys := sortedKeys(fields) lowered := make(map[string]string, len(fields)) var hits []Hit for j := range rs.query { @@ -152,7 +155,7 @@ func (rs *RuleSet) MatchQueryResult(envID uint, environment, queryName string, r if !r.ruleApplies(envID) { continue } - if matched, detail := matchFields(r, fields, lowered); matched { + if matched, detail := matchFields(r, fields, keys, lowered); matched { hits = append(hits, Hit{ RuleID: r.id, RuleName: r.name, @@ -327,7 +330,22 @@ func matchSingle(r *compiledRule, value, key string, lowered map[string]string) // is tried. Values are lowercased at most once per call regardless of // how many rules share the entry — the caller reuses the lowered map // across the whole rule slice. -func matchFields(r *compiledRule, fields map[string]string, lowered map[string]string) (bool, string) { +// sortedKeys returns the keys of fields in a fixed order, for matchFields to +// walk instead of the map: Go randomizes map iteration, and when several fields +// match a rule the one visited first supplies the hit's detail. Computed once +// per entry, not per rule. +func sortedKeys(fields map[string]string) []string { + keys := make([]string, 0, len(fields)) + for k := range fields { + keys = append(keys, k) + } + slices.Sort(keys) + return keys +} + +// keys must be the keys of fields in sorted order (see sortedKeys), so that +// match-any rules report the same detail for the same entry every time. +func matchFields(r *compiledRule, fields map[string]string, keys []string, lowered map[string]string) (bool, string) { lowerOnce := func(k, v string) string { if lv, ok := lowered[k]; ok { return lv @@ -353,14 +371,25 @@ func matchFields(r *compiledRule, fields map[string]string, lowered map[string]s return false, "" } if r.matchRegex != nil { - for _, v := range fields { - if m := r.matchRegex.FindString(v); m != "" { + for _, k := range keys { + if m := r.matchRegex.FindString(fields[k]); m != "" { return true, m } } return false, "" } - for k, v := range fields { + // The empty pattern matches every entry; report the first non-empty + // value, since a blank field (action often is) would say nothing. + if r.matchSubstring == "" { + for _, k := range keys { + if v := fields[k]; v != "" { + return true, v + } + } + return true, "" + } + for _, k := range keys { + v := fields[k] if strings.Contains(lowerOnce(k, v), r.matchSubstring) { return true, substringDetail(r.matchSubstring, v) } diff --git a/pkg/alerts/matcher_test.go b/pkg/alerts/matcher_test.go index 5f24f10b..b108d253 100644 --- a/pkg/alerts/matcher_test.go +++ b/pkg/alerts/matcher_test.go @@ -267,3 +267,28 @@ func makeString(n int) string { } return string(b) } + +// When several columns match, the one visited first supplies the detail. Map +// iteration is random, so the detail must come from a fixed field order. +func TestMatchResultLogsDetailIsDeterministic(t *testing.T) { + rs := compileForTest(t, + AlertRule{Model: ruleWithID(1), Name: "digits", Source: SourceResultLog, MatchType: MatchTypeRegex, MatchValue: `\d+`}, + AlertRule{Model: ruleWithID(2), Name: "anything", Source: SourceResultLog, MatchType: MatchTypeSubstring, MatchValue: ""}, + ) + logs := []types.LogResultData{resultEntry("U", "pack_q", map[string]string{ + "zeta": "999", "beta": "222", "alpha": "111", "gamma": "333", + })} + for i := 0; i < 50; i++ { + hits := rs.MatchResultLogs(NoEnvironmentID, "dev", testNode, logs) + if len(hits) != 2 { + t.Fatalf("expected 2 hits, got %+v", hits) + } + // Sorted keys: action (empty), alpha, beta, gamma, name, zeta. + if hits[0].Detail != "111" { + t.Fatalf("run %d: regex detail = %q, want the first matching column (alpha)", i, hits[0].Detail) + } + if hits[1].Detail != "111" { + t.Fatalf("run %d: empty-pattern detail = %q, want the first non-empty column (alpha)", i, hits[1].Detail) + } + } +} diff --git a/pkg/apiclient/client.go b/pkg/apiclient/client.go index 9f29f07d..d6a19529 100644 --- a/pkg/apiclient/client.go +++ b/pkg/apiclient/client.go @@ -72,6 +72,10 @@ type OsctrlAPI struct { Configuration JSONConfigurationAPI Client *http.Client Headers map[string]string + // UserAgent identifies the calling binary in osctrl-api's logs. Empty + // means the osctrl-cli default; other consumers set their own so their + // traffic can be told apart. + UserAgent string } // LoadConfiguration to load the API configuration file and assign to variables @@ -197,7 +201,11 @@ func (api *OsctrlAPI) ReqGeneric(reqType string, url string, body io.Reader) ([] return []byte{}, fmt.Errorf("NewRequest - %w", err) } // Set custom User-Agent - req.Header.Set(UserAgent, osctrlUserAgent) + ua := api.UserAgent + if ua == "" { + ua = osctrlUserAgent + } + req.Header.Set(UserAgent, ua) // Prepare headers for key, value := range api.Headers { req.Header.Add(key, value) diff --git a/pkg/apiclient/useragent_test.go b/pkg/apiclient/useragent_test.go new file mode 100644 index 00000000..973a9e48 --- /dev/null +++ b/pkg/apiclient/useragent_test.go @@ -0,0 +1,35 @@ +package apiclient + +import ( + "net/http" + "net/http/httptest" + "strings" + "testing" +) + +func TestUserAgent(t *testing.T) { + var got string + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + got = r.Header.Get(UserAgent) + })) + defer srv.Close() + + api, err := CreateAPI(JSONConfigurationAPI{URL: srv.URL, Token: "t"}, false) + if err != nil { + t.Fatalf("CreateAPI: %v", err) + } + if _, err := api.GetGeneric(srv.URL, nil); err != nil { + t.Fatalf("GetGeneric: %v", err) + } + if !strings.HasPrefix(got, "osctrl-cli-http-client/") { + t.Fatalf("default user agent = %q, want the osctrl-cli one", got) + } + + api.UserAgent = "osctrl-mcp/1.2.3" + if _, err := api.GetGeneric(srv.URL, nil); err != nil { + t.Fatalf("GetGeneric: %v", err) + } + if got != "osctrl-mcp/1.2.3" { + t.Fatalf("user agent = %q, want the override", got) + } +} diff --git a/pkg/queries/events_test.go b/pkg/queries/events_test.go index 40165c9a..029e3477 100644 --- a/pkg/queries/events_test.go +++ b/pkg/queries/events_test.go @@ -3,10 +3,10 @@ package queries import ( "fmt" "strings" + "testing" "github.com/jmpsec/osctrl/pkg/events" "github.com/stretchr/testify/require" - "testing" ) type eventRecorder struct{ hints []events.Hint }