From 72f2f7ac127d74999f49243560764095cc0a5bf9 Mon Sep 17 00:00:00 2001 From: gillespi314 <73313222+gillespi314@users.noreply.github.com> Date: Thu, 31 Aug 2023 10:37:51 -0500 Subject: [PATCH] Adjust error messages for run scripts API (#13618) --- ee/server/service/hosts.go | 12 ++--- server/fleet/hosts.go | 8 ++-- server/service/integration_enterprise_test.go | 47 ++++++++++++++----- 3 files changed, 46 insertions(+), 21 deletions(-) diff --git a/ee/server/service/hosts.go b/ee/server/service/hosts.go index aec87765e3..6153e4c107 100644 --- a/ee/server/service/hosts.go +++ b/ee/server/service/hosts.go @@ -62,32 +62,32 @@ func (svc *Service) RunHostScript(ctx context.Context, request *fleet.HostScript // look for the script length in bytes first, as rune counting a huge string // can be expensive. if len(request.ScriptContents) > utf8.UTFMax*maxScriptRuneLen { - return nil, fleet.NewInvalidArgumentError("script_contents", "Error: Script is too large. It's limited to 10,000 characters (approximately 125 lines).") + return nil, fleet.NewInvalidArgumentError("script_contents", "Script is too large. It's limited to 10,000 characters (approximately 125 lines).") } // now that we know that the script is at most 4*maxScriptRuneLen bytes long, // we can safely count the runes for a precise check. if utf8.RuneCountInString(request.ScriptContents) > maxScriptRuneLen { - return nil, fleet.NewInvalidArgumentError("script_contents", "Error: Script is too large. It's limited to 10,000 characters (approximately 125 lines).") + return nil, fleet.NewInvalidArgumentError("script_contents", "Script is too large. It's limited to 10,000 characters (approximately 125 lines).") } // script must be a "text file", but that's not so simple to validate, so we // assume that if it is valid utf8 encoding, it is a text file (binary files // will often have invalid utf8 byte sequences). if !utf8.ValidString(request.ScriptContents) { - return nil, fleet.NewInvalidArgumentError("script_contents", "Error: Wrong data format. Only plain text allowed.") + return nil, fleet.NewInvalidArgumentError("script_contents", "Wrong data format. Only plain text allowed.") } if strings.HasPrefix(request.ScriptContents, "#!") { // read the first line in a portable way s := bufio.NewScanner(strings.NewReader(request.ScriptContents)) // if a hashbang is present, it can only be `/bin/sh` for now if s.Scan() && !scriptHashbangValidation.MatchString(s.Text()) { - return nil, fleet.NewInvalidArgumentError("script_contents", `Error: Interpreter not supported. Bash scripts must run in "#!/bin/sh”.`) + return nil, fleet.NewInvalidArgumentError("script_contents", `Interpreter not supported. Bash scripts must run in "#!/bin/sh”.`) } } // host must be online if host.Status(time.Now()) != fleet.StatusOnline { - return nil, fleet.NewInvalidArgumentError("host_id", "Error: Script can't run on offline host.") + return nil, fleet.NewInvalidArgumentError("host_id", "Script can't run on offline host.") } pending, err := svc.ds.ListPendingHostScriptExecutions(ctx, request.HostID, maxPendingScriptAge) @@ -96,7 +96,7 @@ func (svc *Service) RunHostScript(ctx context.Context, request *fleet.HostScript } if len(pending) > 0 { return nil, fleet.NewInvalidArgumentError( - "script_contents", "Error: A script is already running on this host. Please wait about 1 minute to let it finish.", + "script_contents", "A script is already running on this host. Please wait about 1 minute to let it finish.", ).WithStatus(http.StatusConflict) } diff --git a/server/fleet/hosts.go b/server/fleet/hosts.go index 447b6c6e65..122fb1a626 100644 --- a/server/fleet/hosts.go +++ b/server/fleet/hosts.go @@ -1137,11 +1137,13 @@ func (hsr HostScriptResult) AuthzType() string { func (hsr HostScriptResult) UserMessage(hostTimeout bool) string { switch { case hostTimeout: - return "Error: Fleet hasn't heard from the host in over 1 minute because it went offline. Run the script again when the host comes back online." + return "Fleet hasn't heard from the host in over 1 minute because it went offline. Run the script again when the host comes back online." case !hostTimeout && time.Since(hsr.CreatedAt) > time.Minute: - return "Error: Fleet hasn't heard from the host in over 1 minute because it went offline. Run the script again when the host comes back online." + return "Fleet hasn't heard from the host in over 1 minute because it went offline. Run the script again when the host comes back online." case hsr.ExitCode.Int64 == -1: - return "Error: Timeout. Fleet stopped the script after 30 seconds to protect host performance." + return "Timeout. Fleet stopped the script after 30 seconds to protect host performance." + case hsr.ExitCode.Int64 == -2: + return "Scripts are disabled for this host. To run scripts, deploy a Fleet installer with scripts enabled." case !hsr.ExitCode.Valid: return "Script is running. To see if the script finished, close this modal and open it again." } diff --git a/server/service/integration_enterprise_test.go b/server/service/integration_enterprise_test.go index 9179fc338b..a15cab3650 100644 --- a/server/service/integration_enterprise_test.go +++ b/server/service/integration_enterprise_test.go @@ -3774,6 +3774,7 @@ func (s *integrationEnterpriseTestSuite) TestRunHostScript() { ctx, cancel := context.WithTimeout(ctx, testRunScriptWaitForResult) defer cancel() + resultsCh := make(chan *fleet.HostScriptResultPayload, 1) go func() { for range time.Tick(300 * time.Millisecond) { pending, err := s.ds.ListPendingHostScriptExecutions(ctx, host.ID, 10*time.Second) @@ -3782,22 +3783,28 @@ func (s *integrationEnterpriseTestSuite) TestRunHostScript() { return } if len(pending) > 0 { - // ignoring errors in this goroutine, the HTTP request below will fail if this fails - err = s.ds.SetHostScriptExecutionResult(ctx, &fleet.HostScriptResultPayload{ - HostID: host.ID, - ExecutionID: pending[0].ExecutionID, - Output: "ok", - Runtime: 1, - ExitCode: 0, - }) - if err != nil { - t.Log(err) + select { + case <-ctx.Done(): + return + case r := <-resultsCh: + r.ExecutionID = pending[0].ExecutionID + // ignoring errors in this goroutine, the HTTP request below will fail if this fails + err = s.ds.SetHostScriptExecutionResult(ctx, r) + if err != nil { + t.Log(err) + } } - return } } }() + // simulate a successful script result + resultsCh <- &fleet.HostScriptResultPayload{ + HostID: host.ID, + Output: "ok", + Runtime: 1, + ExitCode: 0, + } runSyncResp = runScriptSyncResponse{} s.DoJSON("POST", "/api/latest/fleet/scripts/run/sync", fleet.HostScriptRequestPayload{HostID: host.ID, ScriptContents: "echo"}, http.StatusOK, &runSyncResp) require.Equal(t, host.ID, runSyncResp.HostID) @@ -3806,7 +3813,23 @@ func (s *integrationEnterpriseTestSuite) TestRunHostScript() { require.True(t, runSyncResp.ExitCode.Valid) require.Equal(t, int64(0), runSyncResp.ExitCode.Int64) require.False(t, runSyncResp.HostTimeout) - require.Empty(t, runSyncResp.Message) + + // simulate a scripts disabled result + resultsCh <- &fleet.HostScriptResultPayload{ + HostID: host.ID, + Output: "", + Runtime: 0, + ExitCode: -2, + } + runSyncResp = runScriptSyncResponse{} + s.DoJSON("POST", "/api/latest/fleet/scripts/run/sync", fleet.HostScriptRequestPayload{HostID: host.ID, ScriptContents: "echo"}, http.StatusOK, &runSyncResp) + require.Equal(t, host.ID, runSyncResp.HostID) + require.NotEmpty(t, runSyncResp.ExecutionID) + require.Empty(t, runSyncResp.Output) + require.True(t, runSyncResp.ExitCode.Valid) + require.Equal(t, int64(-2), runSyncResp.ExitCode.Int64) + require.False(t, runSyncResp.HostTimeout) + require.Contains(t, runSyncResp.Message, "Scripts are disabled") // make the host "offline" err = s.ds.MarkHostsSeen(ctx, []uint{host.ID}, time.Now().Add(-time.Hour))