Bugfix: Only return host timeout error message when script exit code is nil (#15967)

changes/15823-host-timeout-user-msg

@@ -0,0 +1,2 @@
+- Fixed bug where script results would sometimes show the wrong error message when a user attempts
+  to run a script on a host that has scripts disabled.

cmd/fleetctl/scripts_test.go

@@ -176,7 +176,7 @@ Oh no!
 			scriptResult: &fleet.HostScriptResult{
 				ExitCode: ptr.Int64(-2),
 				Output:   "",
-				Message:  "Scripts are disabled for this host. To run scripts, deploy a Fleet installer with scripts enabled.",
+				Message:  fleet.RunScriptDisabledErrMsg,
 			},
 			expectOutput: `
 Error: Scripts are disabled for this host. To run scripts, deploy a Fleet installer with scripts enabled.

server/fleet/errors.go

@@ -543,6 +543,7 @@ const (
 	RunScriptAlreadyRunningErrMsg          = "A script is already running on this host. Please wait about 5 minutes to let it finish."
 	RunScriptHostTimeoutErrMsg             = "Fleet hasn’t heard from the host in over 5 minutes. Fleet doesn’t know if the script ran because the host went offline."
 	RunScriptScriptsDisabledGloballyErrMsg = "Running scripts is disabled in organization settings."
+	RunScriptDisabledErrMsg                = "Scripts are disabled for this host. To run scripts, deploy a Fleet installer with scripts enabled."
 	RunScriptScriptTimeoutErrMsg           = "Timeout. Fleet stopped the script after 5 minutes to protect host performance."
 )
 

server/fleet/scripts.go

@@ -215,14 +215,14 @@ func (hsr HostScriptResult) UserMessage(hostTimeout bool) string {
 	case -1:
 		return RunScriptScriptTimeoutErrMsg
 	case -2:
-		return "Scripts are disabled for this host. To run scripts, deploy a Fleet installer with scripts enabled."
+		return RunScriptDisabledErrMsg
 	default:
 		return ""
 	}
 }
 
 func (hsr HostScriptResult) HostTimeout(waitForResultTime time.Duration) bool {
-	return time.Now().After(hsr.CreatedAt.Add(waitForResultTime))
+	return hsr.ExitCode == nil && time.Now().After(hsr.CreatedAt.Add(waitForResultTime))
 }
 
 const MaxScriptRuneLen = 10000

server/service/integration_enterprise_test.go

@@ -5444,6 +5444,84 @@ VALUES
 		require.NotNil(t, resp.Scripts)
 		require.Len(t, resp.Scripts, 0)
 	})
+
+	t.Run("get script results user message", func(t *testing.T) {
+		// add a script with an older created_at timestamp
+		var oldScriptID uint
+		mysql.ExecAdhocSQL(t, s.ds, func(tx sqlx.ExtContext) error {
+			res, err := tx.ExecContext(ctx, `
+INSERT INTO
+	scripts (name, script_contents, created_at, updated_at)
+VALUES
+	(?,?,?,?)`,
+				"test-script-details-timeout.sh",
+				"echo test-script-details-timeout",
+				now.Add(-1*time.Hour),
+				now.Add(-1*time.Hour),
+			)
+			if err != nil {
+				return err
+			}
+			id, err := res.LastInsertId()
+			if err != nil {
+				return err
+			}
+			oldScriptID = uint(id)
+			return nil
+		})
+
+		for _, c := range []struct {
+			name       string
+			exitCode   *int64
+			executedAt time.Time
+			expected   string
+		}{
+			{
+				name:       "host-timeout",
+				exitCode:   nil,
+				executedAt: now.Add(-1 * time.Hour),
+				expected:   fleet.RunScriptHostTimeoutErrMsg,
+			},
+			{
+				name:       "script-timeout",
+				exitCode:   ptr.Int64(-1),
+				executedAt: now.Add(-1 * time.Hour),
+				expected:   fleet.RunScriptScriptTimeoutErrMsg,
+			},
+			{
+				name:       "pending",
+				exitCode:   nil,
+				executedAt: now.Add(-1 * time.Minute),
+				expected:   fleet.RunScriptAlreadyRunningErrMsg,
+			},
+			{
+				name:       "success",
+				exitCode:   ptr.Int64(0),
+				executedAt: now.Add(-1 * time.Hour),
+				expected:   "",
+			},
+			{
+				name:       "error",
+				exitCode:   ptr.Int64(1),
+				executedAt: now.Add(-1 * time.Hour),
+				expected:   "",
+			},
+			{
+				name:       "disabled",
+				exitCode:   ptr.Int64(-2),
+				executedAt: now.Add(-1 * time.Hour),
+				expected:   fleet.RunScriptDisabledErrMsg,
+			},
+		} {
+			t.Run(c.name, func(t *testing.T) {
+				insertResults(t, host0.ID, &fleet.Script{ID: oldScriptID, Name: "test-script-details-timeout.sh"}, c.executedAt, "test-user-message_"+c.name, c.exitCode)
+
+				var resp getScriptResultResponse
+				s.DoJSON("GET", fmt.Sprintf("/api/latest/fleet/scripts/results/%s", "test-user-message_"+c.name), nil, http.StatusOK, &resp)
+				require.Equal(t, c.expected, resp.Message)
+			})
+		}
+	})
 }
 
 // generates the body and headers part of a multipart request ready to be

server/service/scripts.go

@@ -147,7 +147,8 @@ func getScriptResultEndpoint(ctx context.Context, request interface{}, svc fleet
 		return getScriptResultResponse{Err: err}, nil
 	}
 
-	// check if a minute has passed since the script was created at
+	// TODO: move this logic out of the endpoint function and consolidate in either the service
+	// method or the fleet package
 	hostTimeout := scriptResult.HostTimeout(scripts.MaxServerWaitTime)
 	scriptResult.Message = scriptResult.UserMessage(hostTimeout)