From b4a2115b2cd8c39918ade8d8889c322c578c6f11 Mon Sep 17 00:00:00 2001 From: Dante Catalfamo <43040593+dantecatalfamo@users.noreply.github.com> Date: Fri, 10 Jan 2025 13:13:28 -0500 Subject: [PATCH] Display correct key path to user for agent options (#25199) #24038 --- changes/24038-agent-options-key-error | 1 + ee/server/service/teams.go | 17 ++++ server/fleet/agent_options.go | 85 +++++++++++++++++++ server/fleet/errors.go | 9 ++ server/service/integration_enterprise_test.go | 26 ++++++ 5 files changed, 138 insertions(+) create mode 100644 changes/24038-agent-options-key-error diff --git a/changes/24038-agent-options-key-error b/changes/24038-agent-options-key-error new file mode 100644 index 0000000000..dd8235c59a --- /dev/null +++ b/changes/24038-agent-options-key-error @@ -0,0 +1 @@ +- Display the correct path for agent options when a key is placed in the wrong object diff --git a/ee/server/service/teams.go b/ee/server/service/teams.go index eded9b4788..121351b255 100644 --- a/ee/server/service/teams.go +++ b/ee/server/service/teams.go @@ -414,6 +414,23 @@ func (svc *Service) ModifyTeamAgentOptions(ctx context.Context, teamID uint, tea if teamOptions != nil { if err := fleet.ValidateJSONAgentOptions(ctx, svc.ds, teamOptions, true); err != nil { + if field := fleet.GetJSONUnknownField(err); field != nil { + correctKeyPath, keyErr := fleet.FindAgentOptionsKeyPath(*field) + if keyErr != nil { + level.Error(svc.logger).Log("err", err, "msg", "error parsing generated agent options structs") + } + var keyPathJoined string + switch pathLen := len(correctKeyPath); { + case pathLen > 1: + keyPathJoined = fmt.Sprintf("%q", strings.Join(correctKeyPath[:len(correctKeyPath)-1], ".")) + case pathLen == 1: + keyPathJoined = "top level" + } + if keyPathJoined != "" { + err = fmt.Errorf("%q should be part of the %s object", *field, keyPathJoined) + } + } + err = fleet.NewUserMessageError(err, http.StatusBadRequest) if applyOptions.Force && !applyOptions.DryRun { level.Info(svc.logger).Log("err", err, "msg", "force-apply team agent options with validation errors") diff --git a/server/fleet/agent_options.go b/server/fleet/agent_options.go index 0e185a3fee..51f9d94dcf 100644 --- a/server/fleet/agent_options.go +++ b/server/fleet/agent_options.go @@ -325,3 +325,88 @@ func validateJSONAgentOptionsSet(rawJSON json.RawMessage) error { } return nil } + +func FindAgentOptionsKeyPath(key string) ([]string, error) { + if key == "script_execution_timeout" { + return []string{"script_execution_timeout"}, nil + } + + configPath, err := locateStructJSONKeyPath(key, "config", osqueryAgentOptions{}) + if err != nil { + return nil, fmt.Errorf("locating key path in agent options: %w", err) + } + if configPath != nil { + return configPath, nil + } + + if key == "overrides" { + return []string{"overrides"}, nil + } + if key == "platforms" { + return []string{"overrides", "platforms"}, nil + } + + commandLinePath, err := locateStructJSONKeyPath(key, "command_line_flags", osqueryCommandLineFlags{}) + if err != nil { + return nil, fmt.Errorf("locating key path in agent command line options: %w", err) + } + if commandLinePath != nil { + return commandLinePath, nil + } + + extensionsPath, err := locateStructJSONKeyPath(key, "extensions", ExtensionInfo{}) + if err != nil { + return nil, fmt.Errorf("locating key path in agent extensions options: %w", err) + } + if extensionsPath != nil { + return extensionsPath, nil + } + + channelsPath, err := locateStructJSONKeyPath(key, "update_channels", OrbitUpdateChannels{}) + if err != nil { + return nil, fmt.Errorf("locating key path in agent update channels: %w", err) + } + if channelsPath != nil { + return channelsPath, nil + } + + return nil, nil +} + +// Only searches two layers deep +func locateStructJSONKeyPath(key, startKey string, target any) ([]string, error) { + if key == startKey { + return []string{startKey}, nil + } + + optionsBytes, err := json.Marshal(target) + if err != nil { + return nil, fmt.Errorf("unable to marshall target: %w", err) + } + + var opts map[string]any + + if err := json.Unmarshal(optionsBytes, &opts); err != nil { + return nil, fmt.Errorf("unable to unmarshall target: %w", err) + } + + var path [3]string + path[0] = startKey + for k, v := range opts { + path[1] = k + if k == key { + return path[:2], nil + } + + if inner, ok := v.(map[string]any); ok { + for k2 := range inner { + path[2] = k2 + if key == k2 { + return path[:3], nil + } + } + } + } + + return nil, nil +} diff --git a/server/fleet/errors.go b/server/fleet/errors.go index fb5e302c11..77098617bc 100644 --- a/server/fleet/errors.go +++ b/server/fleet/errors.go @@ -477,6 +477,15 @@ func IsJSONUnknownFieldError(err error) bool { return rxJSONUnknownField.MatchString(err.Error()) } +func GetJSONUnknownField(err error) *string { + errCause := Cause(err) + if IsJSONUnknownFieldError(errCause) { + substr := rxJSONUnknownField.FindStringSubmatch(errCause.Error()) + return &substr[1] + } + return nil +} + // UserMessage implements the user-friendly translation of the error if its // root cause is one of the supported types, otherwise it returns the error // message. diff --git a/server/service/integration_enterprise_test.go b/server/service/integration_enterprise_test.go index 8d102a9746..b210cb1e8f 100644 --- a/server/service/integration_enterprise_test.go +++ b/server/service/integration_enterprise_test.go @@ -1476,6 +1476,32 @@ func (s *integrationEnterpriseTestSuite) TestTeamEndpoints() { "x": "y" }`), http.StatusBadRequest, &tmResp) + // modify team agent options with invalid key + badRes := s.Do("POST", fmt.Sprintf("/api/latest/fleet/teams/%d/agent_options", tm1ID), json.RawMessage(`{ + "bad_key": 1 + }`), http.StatusBadRequest) + errText := extractServerErrorText(badRes.Body) + require.Contains(t, errText, "unsupported key provided") + + // modify team agent options with correct options under the wrong key + badRes = s.Do("POST", fmt.Sprintf("/api/latest/fleet/teams/%d/agent_options", tm1ID), json.RawMessage(`{ + "distributed_tls_max_attempts": 3 + }`), http.StatusBadRequest) + errText = extractServerErrorText(badRes.Body) + require.Contains(t, errText, "\"distributed_tls_max_attempts\" should be part of the \"config.options\" object") + + badRes = s.Do("POST", fmt.Sprintf("/api/latest/fleet/teams/%d/agent_options", tm1ID), json.RawMessage(`{ + "config": { "options": { "logger_plugin": 3 } } + }`), http.StatusBadRequest) + errText = extractServerErrorText(badRes.Body) + require.Contains(t, errText, "\"logger_plugin\" should be part of the \"command_line_flags\" object") + + badRes = s.Do("POST", fmt.Sprintf("/api/latest/fleet/teams/%d/agent_options", tm1ID), json.RawMessage(`{ + "update_channels": { "config": 1 } + }`), http.StatusBadRequest) + errText = extractServerErrorText(badRes.Body) + require.Contains(t, errText, "\"config\" should be part of the top level object") + // modify team agent options with invalid platform options tmResp.Team = nil s.DoJSON("POST", fmt.Sprintf("/api/latest/fleet/teams/%d/agent_options", tm1ID), json.RawMessage(