Discharge7443/fleet
1
0
Fork 0
Code Issues Pull Requests Actions 8 Packages Projects Releases Wiki Activity

Adjust response payload, messages and validations for /scripts/run/* endpoints. (#13607)

Browse Source
This commit is contained in:
Martin Angers
2023-08-31 09:08:50 -05:00
committed by GitHub
parent 9142c5de79
commit cbc3f32e9d
9 changed files with 75 additions and 41 deletions
Download Patch File Download Diff File Expand all files Collapse all files
cmd/osquery-perf/agent.go
+1 -1
View File
@@ -625,7 +625,7 @@ func (a *agent) execScripts(execIDs []string, orbitClient *service.OrbitClient)
// send a no-op result without executing if script exec is disabled
if err := orbitClient.SaveHostScriptResult(&fleet.HostScriptResultPayload{
ExecutionID: execID,
Output: "script execution disabled",
Output: "Scripts are disabled",
Runtime: 0,
ExitCode: -2,
}); err != nil {
docs/REST API/rest-api.md
+2
View File
@@ -6212,7 +6212,9 @@ Creates a script execution request and waits for a result to return (up to a 1 m
"execution_id": "e797d6c6-3aae-11ee-be56-0242ac120002",
"script_contents": "echo 'hello'",
"output": "hello",
"message": "",
"runtime": 1,
"host_timeout": false,
"exit_code": 0
}
```
ee/server/service/hosts.go
+13 -18
View File
@@ -3,7 +3,7 @@ package service
import (
"bufio"
"context"
"fmt"
"net/http"
"regexp"
"strings"
"time"
@@ -62,33 +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", fmt.Sprintf("script is too long, must be at most %d characters", maxScriptRuneLen))
return nil, fleet.NewInvalidArgumentError("script_contents", "Error: 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", fmt.Sprintf("script is too long, must be at most %d characters", maxScriptRuneLen))
return nil, fleet.NewInvalidArgumentError("script_contents", "Error: 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", "script must be a valid utf8-encoded text file")
return nil, fleet.NewInvalidArgumentError("script_contents", "Error: 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", "script cannot start with a hashbang (#!) other than #!/bin/sh")
return nil, fleet.NewInvalidArgumentError("script_contents", `Error: Interpreter not supported. Bash scripts must run in "#!/bin/sh”.`)
}
}
// host must be online if a "sync" script execution is requested (i.e. if we
// will poll to get and return results).
if waitForResult > 0 && host.Status(time.Now()) != fleet.StatusOnline {
return nil, fleet.NewInvalidArgumentError("host_id", "host is offline")
// 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.")
}
pending, err := svc.ds.ListPendingHostScriptExecutions(ctx, request.HostID, maxPendingScriptAge)
@@ -96,15 +95,9 @@ func (svc *Service) RunHostScript(ctx context.Context, request *fleet.HostScript
return nil, ctxerr.Wrap(ctx, err, "list host pending script executions")
}
if len(pending) > 0 {
// TODO(mna): there are a number of issues with that validation: it only
// really says that there was a script execution _request_ that was made < 1m
// ago, and that blocks executing any more scripts on that host, but the
// host may not even have received the previous script for execution yet,
// so if we accept more scripts after 1m, we may end up having multiple
// scripts to execute on the host at the same time (or more likely in
// sequence, but still). This may be good enough for now, I think the whole
// idea of locking if a script is pending is meant to be temporary anyway.
return nil, fleet.NewInvalidArgumentError("script_contents", "a script is currently executing on the host")
return nil, fleet.NewInvalidArgumentError(
"script_contents", "Error: A script is already running on this host. Please wait about 1 minute to let it finish.",
).WithStatus(http.StatusConflict)
}
// create the script execution request, the host will be notified of the
@@ -113,6 +106,7 @@ func (svc *Service) RunHostScript(ctx context.Context, request *fleet.HostScript
if err != nil {
return nil, ctxerr.Wrap(ctx, err, "create script execution request")
}
script.Hostname = host.DisplayName()
if waitForResult <= 0 {
// async execution, return
@@ -143,6 +137,7 @@ func (svc *Service) RunHostScript(ctx context.Context, request *fleet.HostScript
}
if result.ExitCode.Valid {
// a result was received from the host, return
result.Hostname = host.DisplayName()
return result, nil
}
orbit/pkg/scripts/scripts.go
+1 -1
View File
@@ -171,7 +171,7 @@ func (r *Runner) createRunDir(execID string) (string, error) {
func (r *Runner) runOneDisabled(execID string) error {
err := r.Client.SaveHostScriptResult(&fleet.HostScriptResultPayload{
ExecutionID: execID,
Output: "script execution disabled",
Output: "Scripts are disabled",
ExitCode: -2, // fleetctl knows that -2 means script was disabled on host
})
if err != nil {
server/datastore/mysql/hosts.go
+3 -2
View File
@@ -4183,7 +4183,7 @@ func (ds *Datastore) GetMatchingHostSerials(ctx context.Context, serials []strin
func (ds *Datastore) NewHostScriptExecutionRequest(ctx context.Context, request *fleet.HostScriptRequestPayload) (*fleet.HostScriptResult, error) {
const (
insStmt = `INSERT INTO host_script_results (host_id, execution_id, script_contents, output) VALUES (?, ?, ?, '')`
getStmt = `SELECT id, host_id, execution_id, script_contents FROM host_script_results WHERE id = ?`
getStmt = `SELECT id, host_id, execution_id, script_contents, created_at FROM host_script_results WHERE id = ?`
)
execID := uuid.New().String()
@@ -4269,7 +4269,8 @@ func (ds *Datastore) GetHostScriptExecutionResult(ctx context.Context, execID st
script_contents,
output,
runtime,
exit_code
exit_code,
created_at
FROM
host_script_results
WHERE
server/fleet/hosts.go
+29
View File
@@ -1109,12 +1109,41 @@ type HostScriptResult struct {
// host. It is -1 if it was received but the script did not terminate
// normally (same as how Go handles this: https://pkg.go.dev/os#ProcessState.ExitCode)
ExitCode sql.NullInt64 `json:"exit_code" db:"exit_code"`
// CreatedAt is the creation timestamp of the script execution request. It is
// not returned as part of the payloads, but is used to determine if the script
// is too old to still expect a response from the host.
CreatedAt time.Time `json:"-" db:"created_at"`
// TeamID is only used for authorization, it must be set to the team id of
// the host when checking authorization and is otherwise not set.
TeamID *uint `json:"team_id" db:"-"`
// Hostname can be set by the endpoint as extra information to make available
// when generating the UserMessage associated with a response from an
// execution. It is otherwise not part of the host_script_results table and
// not returned as part of the resulting JSON.
Hostname string `json:"-" db:"-"`
}
func (hsr HostScriptResult) AuthzType() string {
return "host_script_result"
}
// UserMessage returns the user-friendly message to associate with the current
// state of the HostScriptResult. This is returned as part of the API endpoints
// for running a script synchronously (so that fleetctl can display it) and to
// get the script results for an execution ID (e.g. when looking at the details
// screen of a script execution activity in the website).
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."
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."
case hsr.ExitCode.Int64 == -1:
return "Error: Timeout. Fleet stopped the script after 30 seconds to protect host performance."
case !hsr.ExitCode.Valid:
return "Script is running. To see if the script finished, close this modal and open it again."
}
return ""
}
server/service/hosts.go
+7 -8
View File
@@ -1632,13 +1632,13 @@ type runScriptSyncResponse struct {
Err error `json:"error,omitempty"`
*fleet.HostScriptResult
// only set if the error was a timeout waiting for a result
ErrorMessage string `json:"error_message,omitempty"`
Message string `json:"message"`
HostTimeout bool `json:"host_timeout"`
}
func (r runScriptSyncResponse) error() error { return r.Err }
func (r runScriptSyncResponse) Status() int {
if r.ErrorMessage != "" {
if r.HostTimeout {
return http.StatusGatewayTimeout
}
return http.StatusOK
@@ -1660,23 +1660,22 @@ func runScriptSyncEndpoint(ctx context.Context, request interface{}, svc fleet.S
}, waitForResult)
if err != nil {
if errors.Is(err, context.DeadlineExceeded) {
err = fleet.NewGatewayTimeoutError("script execution timed out waiting for a result", err)
// it should still return the execution id and host id in this situation,
// so the user knows what script request to look at in the UI. We cannot
// return an error (field Err) in this case, as the errorer interface's
// rendering logic would take over and only render the error part of the
// response struct. This is why we use the distinct ErrorMessage field to
// add the error message and status code to the response, along with the
// script request.
// response struct.
return runScriptSyncResponse{
HostScriptResult: result,
ErrorMessage: err.Error(),
HostTimeout: true,
Message: result.UserMessage(true),
}, nil
}
return runScriptSyncResponse{Err: err}, nil
}
return runScriptSyncResponse{
HostScriptResult: result,
Message: result.UserMessage(false),
}, nil
}
server/service/hosts_test.go
+4 -4
View File
@@ -1242,14 +1242,14 @@ func TestHostRunScript(t *testing.T) {
wantErr string
}{
{"empty script", "", "a script to execute is required"},
{"overly long script", strings.Repeat("a", 10001), "script is too long"},
{"invalid utf8", "\xff\xfa", "must be a valid utf8-encoded text file"},
{"overly long script", strings.Repeat("a", 10001), "Script is too large."},
{"invalid utf8", "\xff\xfa", "Wrong data format."},
{"valid without hashbang", "echo 'a'", ""},
{"valid with hashbang", "#!/bin/sh\necho 'a'", ""},
{"valid with hashbang and spacing", "#! /bin/sh \necho 'a'", ""},
{"valid with hashbang and Windows newline", "#! /bin/sh \r\necho 'a'", ""},
{"invalid hashbang", "#!/bin/bash\necho 'a'", "cannot start with a hashbang"},
{"invalid hashbang suffix", "#!/bin/sh -n\necho 'a'", "cannot start with a hashbang"},
{"invalid hashbang", "#!/bin/bash\necho 'a'", "Interpreter not supported."},
{"invalid hashbang suffix", "#!/bin/sh -n\necho 'a'", "Interpreter not supported."},
}
ctx = viewer.NewContext(ctx, viewer.Viewer{User: test.UserAdmin})
server/service/integration_enterprise_test.go
+15 -7
View File
@@ -3675,7 +3675,7 @@ func (s *integrationEnterpriseTestSuite) TestRunHostScript() {
// attempt to run an overly long script
res = s.Do("POST", "/api/latest/fleet/scripts/run", fleet.HostScriptRequestPayload{HostID: host.ID, ScriptContents: strings.Repeat("a", 10001)}, http.StatusUnprocessableEntity)
errMsg = extractServerErrorText(res.Body)
require.Contains(t, errMsg, "script is too long")
require.Contains(t, errMsg, "Script is too large.")
// make sure the host is still seen as "online"
err := s.ds.MarkHostsSeen(ctx, []uint{host.ID}, time.Now())
@@ -3730,7 +3730,7 @@ func (s *integrationEnterpriseTestSuite) TestRunHostScript() {
// attempt to sync run an overly long script
res = s.Do("POST", "/api/latest/fleet/scripts/run/sync", fleet.HostScriptRequestPayload{HostID: host.ID, ScriptContents: strings.Repeat("a", 10001)}, http.StatusUnprocessableEntity)
errMsg = extractServerErrorText(res.Body)
require.Contains(t, errMsg, "script is too long")
require.Contains(t, errMsg, "Script is too large.")
// make sure the host is still seen as "online"
err = s.ds.MarkHostsSeen(ctx, []uint{host.ID}, time.Now())
@@ -3738,9 +3738,9 @@ func (s *integrationEnterpriseTestSuite) TestRunHostScript() {
// attempt to create a valid sync script execution request, fails because the
// host has a pending script execution
res = s.Do("POST", "/api/latest/fleet/scripts/run/sync", fleet.HostScriptRequestPayload{HostID: host.ID, ScriptContents: "echo"}, http.StatusUnprocessableEntity)
res = s.Do("POST", "/api/latest/fleet/scripts/run/sync", fleet.HostScriptRequestPayload{HostID: host.ID, ScriptContents: "echo"}, http.StatusConflict)
errMsg = extractServerErrorText(res.Body)
require.Contains(t, errMsg, "a script is currently executing on the host")
require.Contains(t, errMsg, "A script is already running on this host.")
// save a result via the orbit endpoint
var orbitPostScriptResp orbitPostScriptResultResponse
@@ -3761,7 +3761,8 @@ func (s *integrationEnterpriseTestSuite) TestRunHostScript() {
s.DoJSON("POST", "/api/latest/fleet/scripts/run/sync", fleet.HostScriptRequestPayload{HostID: host.ID, ScriptContents: "echo"}, http.StatusGatewayTimeout, &runSyncResp)
require.Equal(t, host.ID, runSyncResp.HostID)
require.NotEmpty(t, runSyncResp.ExecutionID)
require.Contains(t, runSyncResp.ErrorMessage, "script execution timed out waiting for a result")
require.True(t, runSyncResp.HostTimeout)
require.Contains(t, runSyncResp.Message, "Fleet hasn't heard from the host in over 1 minute because it went offline.")
s.DoJSON("POST", "/api/fleet/orbit/scripts/result",
json.RawMessage(fmt.Sprintf(`{"orbit_node_key": %q, "execution_id": %q, "exit_code": 0, "output": "ok"}`, *host.OrbitNodeKey, runSyncResp.ExecutionID)),
@@ -3804,7 +3805,8 @@ func (s *integrationEnterpriseTestSuite) TestRunHostScript() {
require.Equal(t, "ok", runSyncResp.Output)
require.True(t, runSyncResp.ExitCode.Valid)
require.Equal(t, int64(0), runSyncResp.ExitCode.Int64)
require.Empty(t, runSyncResp.ErrorMessage)
require.False(t, runSyncResp.HostTimeout)
require.Empty(t, runSyncResp.Message)
// make the host "offline"
err = s.ds.MarkHostsSeen(ctx, []uint{host.ID}, time.Now().Add(-time.Hour))
@@ -3814,5 +3816,11 @@ func (s *integrationEnterpriseTestSuite) TestRunHostScript() {
// is offline.
res = s.Do("POST", "/api/latest/fleet/scripts/run/sync", fleet.HostScriptRequestPayload{HostID: host.ID, ScriptContents: "echo"}, http.StatusUnprocessableEntity)
errMsg = extractServerErrorText(res.Body)
require.Contains(t, errMsg, "host is offline")
require.Contains(t, errMsg, "Script can't run on offline host.")
// attempt to create an async script execution request, fails because the host
// is offline.
res = s.Do("POST", "/api/latest/fleet/scripts/run", fleet.HostScriptRequestPayload{HostID: host.ID, ScriptContents: "echo"}, http.StatusUnprocessableEntity)
errMsg = extractServerErrorText(res.Body)
require.Contains(t, errMsg, "Script can't run on offline host.")
}