missing validations and tweaks to default scripts (#18780)

This adds two things:

- when implementing the CLI, I found [a
panel](https://www.figma.com/file/oQl2oQUG0iRkUy0YOxc307/%2314921-Deploy-security-agents-to-macOS%2C-Windows%2C-and-Linux-hosts?type=design&node-id=779-29335&mode=design&t=Y27cbj7DdhUEGJko-4)
in the Figma file with validations that I missed
- explicit shebang for bash scrips (requested by product) and removed a
comment that will be user facing for exe files.

ee/server/service/software_installers.go

@@ -4,7 +4,9 @@ import (
 	"context"
 	"encoding/hex"
 	"errors"
+	"fmt"
 	"net/http"
+	"path/filepath"
 	"strings"
 
 	"github.com/fleetdm/fleet/v4/pkg/file"
@@ -219,19 +221,47 @@ func (svc *Service) InstallSoftwareTitle(ctx context.Context, hostID uint, softw
 		return err
 	}
 
-	err = svc.ds.InsertSoftwareInstallRequest(ctx, hostID, softwareTitleID, host.TeamID)
+	installer, err := svc.ds.GetSoftwareInstallerForTitle(ctx, softwareTitleID, host.TeamID)
 	if err != nil {
 		if fleet.IsNotFound(err) {
 			return &fleet.BadRequestError{
-				Message:     "The software title provided doesn't have an installer",
-				InternalErr: ctxerr.Wrapf(ctx, err, "couldn't find an installer for software title"),
+				Message: "Software title has no package added. Please add software package to install.",
+				InternalErr: ctxerr.WrapWithData(
+					ctx, err, "couldn't find an installer for software title",
+					map[string]any{"host_id": host.ID, "team_id": host.TeamID, "title_id": softwareTitleID},
+				),
 			}
 		}
 
-		return ctxerr.Wrap(ctx, err, "inserting software install request")
+		return ctxerr.Wrap(ctx, err, "finding software installer for title")
 	}
 
-	return nil
+	ext := filepath.Ext(installer.Name)
+	var requiredPlatform string
+	switch ext {
+	case ".msi", ".exe":
+		requiredPlatform = "windows"
+	case ".pkg":
+		requiredPlatform = "darwin"
+	case ".deb":
+		requiredPlatform = "linux"
+	default:
+		// this should never happen
+		return ctxerr.Errorf(ctx, "software installer has unsupported type %s", ext)
+	}
+
+	if host.FleetPlatform() != requiredPlatform {
+		return &fleet.BadRequestError{
+			Message: fmt.Sprintf("Package (%s) can be installed only on %s hosts.", ext, requiredPlatform),
+			InternalErr: ctxerr.WrapWithData(
+				ctx, err, "invalid host platform for requested installer",
+				map[string]any{"host_id": host.ID, "team_id": host.TeamID, "title_id": softwareTitleID},
+			),
+		}
+	}
+
+	err = svc.ds.InsertSoftwareInstallRequest(ctx, hostID, installer.InstallerID)
+	return ctxerr.Wrap(ctx, err, "inserting software install request")
 }
 
 func (svc *Service) GetSoftwareInstallResults(ctx context.Context, resultUUID string) (*fleet.HostSoftwareInstallerResult, error) {

pkg/file/scripts/install_deb.sh

@@ -1 +1,3 @@
+#!/bin/sh
+
 apt-get install -f "$INSTALLER_PATH"

pkg/file/scripts/install_exe.ps1

@@ -4,11 +4,6 @@ $exeFilePath = "$INSTALLER_PATH"
 $exeName = [System.IO.Path]::GetFileName($exeFilePath)
 $subDir = [System.IO.Path]::GetFileNameWithoutExtension($exeFilePath)
 
-# Program Files is the recommended location for any third-party software on Windows.
-#
-# Note: a x86 binary on a x64 system is supposed to go in
-# $env:ProgramFiles(x86) but I didn't find a reliable way to get this
-# information from the exe file.
 $destinationPath = Join-Path -Path $env:ProgramFiles -ChildPath $subDir
 
 # check if the directory does not exist, and create it if necessary

pkg/file/scripts/install_pkg.sh

@@ -1 +1,3 @@
+#!/bin/sh
+
 installer -pkg "$INSTALLER_PATH" -target /

pkg/file/scripts/remove_deb.sh

@@ -1 +1,3 @@
+#!/bin/sh
+
 apt-get remove -y $(dpkg -f "$INSTALLER_PATH" Package)

pkg/file/scripts/remove_pkg.sh

@@ -1,3 +1,5 @@
+#!/bin/sh
+
 # grab the identifier from the first PackageInfo we find. Those are placed in different locations depending on the installer
 pkg_id=$(tar xOvf "$INSTALLER_PATH" --include='*PackageInfo*' 2>/dev/null | sed -n 's/.*identifier="\([^"]*\)".*/\1/p')
 

pkg/file/testdata/scripts/install_deb.sh.golden

@@ -1 +1,3 @@
+#!/bin/sh
+
 apt-get install -f "$INSTALLER_PATH"

pkg/file/testdata/scripts/install_exe.ps1.golden

@@ -4,11 +4,6 @@ $exeFilePath = "$INSTALLER_PATH"
 $exeName = [System.IO.Path]::GetFileName($exeFilePath)
 $subDir = [System.IO.Path]::GetFileNameWithoutExtension($exeFilePath)
 
-# Program Files is the recommended location for any third-party software on Windows.
-#
-# Note: a x86 binary on a x64 system is supposed to go in
-# $env:ProgramFiles(x86) but I didn't find a reliable way to get this
-# information from the exe file.
 $destinationPath = Join-Path -Path $env:ProgramFiles -ChildPath $subDir
 
 # check if the directory does not exist, and create it if necessary

pkg/file/testdata/scripts/install_pkg.sh.golden

@@ -1 +1,3 @@
+#!/bin/sh
+
 installer -pkg "$INSTALLER_PATH" -target /

pkg/file/testdata/scripts/remove_deb.sh.golden

@@ -1 +1,3 @@
+#!/bin/sh
+
 apt-get remove -y $(dpkg -f "$INSTALLER_PATH" Package)

pkg/file/testdata/scripts/remove_pkg.sh.golden

@@ -1,3 +1,5 @@
+#!/bin/sh
+
 # grab the identifier from the first PackageInfo we find. Those are placed in different locations depending on the installer
 pkg_id=$(tar xOvf "$INSTALLER_PATH" --include='*PackageInfo*' 2>/dev/null | sed -n 's/.*identifier="\([^"]*\)".*/\1/p')
 

server/datastore/mysql/activities_test.go

@@ -382,10 +382,10 @@ func testListHostUpcomingActivities(t *testing.T, ds *Datastore) {
 	require.NoError(t, err)
 	h1E := hsr.ExecutionID
 	// create some software installs requests for h1, make some complete
-	err = ds.InsertSoftwareInstallRequest(ctx, h1.ID, sw1Meta.TitleID, nil)
+	err = ds.InsertSoftwareInstallRequest(ctx, h1.ID, sw1Meta.InstallerID)
 	require.NoError(t, err)
 	h1FooFailed := latestSoftwareInstallerUUID()
-	err = ds.InsertSoftwareInstallRequest(ctx, h1.ID, sw2Meta.TitleID, nil)
+	err = ds.InsertSoftwareInstallRequest(ctx, h1.ID, sw2Meta.InstallerID)
 	require.NoError(t, err)
 	h1Bar := latestSoftwareInstallerUUID()
 	err = ds.SetHostSoftwareInstallResult(ctx, &fleet.HostSoftwareInstallResultPayload{
@@ -394,7 +394,7 @@ func testListHostUpcomingActivities(t *testing.T, ds *Datastore) {
 		PreInstallConditionOutput: ptr.String(""), // pre-install failed
 	})
 	require.NoError(t, err)
-	err = ds.InsertSoftwareInstallRequest(ctx, h1.ID, sw1Meta.TitleID, nil)
+	err = ds.InsertSoftwareInstallRequest(ctx, h1.ID, sw1Meta.InstallerID)
 	require.NoError(t, err)
 	h1FooInstalled := latestSoftwareInstallerUUID()
 	err = ds.SetHostSoftwareInstallResult(ctx, &fleet.HostSoftwareInstallResultPayload{
@@ -404,7 +404,7 @@ func testListHostUpcomingActivities(t *testing.T, ds *Datastore) {
 		InstallScriptExitCode:     ptr.Int(0),
 	})
 	require.NoError(t, err)
-	err = ds.InsertSoftwareInstallRequest(noUserCtx, h1.ID, sw1Meta.TitleID, nil) // no user for this one
+	err = ds.InsertSoftwareInstallRequest(noUserCtx, h1.ID, sw1Meta.InstallerID) // no user for this one
 	require.NoError(t, err)
 	h1Foo := latestSoftwareInstallerUUID()
 
@@ -418,7 +418,7 @@ func testListHostUpcomingActivities(t *testing.T, ds *Datastore) {
 	require.NoError(t, err)
 	h2F := hsr.ExecutionID
 	// add a pending software install request for h2
-	err = ds.InsertSoftwareInstallRequest(ctx, h2.ID, sw2Meta.TitleID, nil)
+	err = ds.InsertSoftwareInstallRequest(ctx, h2.ID, sw2Meta.InstallerID)
 	require.NoError(t, err)
 	h2Bar := latestSoftwareInstallerUUID()
 

server/datastore/mysql/software_installers.go

@@ -192,12 +192,43 @@ func (ds *Datastore) DeleteSoftwareInstaller(ctx context.Context, id uint) error
 	return nil
 }
 
-func (ds *Datastore) InsertSoftwareInstallRequest(ctx context.Context, hostID uint, softwareTitleID uint, teamID *uint) error {
+func (ds *Datastore) GetSoftwareInstallerForTitle(ctx context.Context, softwareTitleID uint, teamID *uint) (*fleet.SoftwareInstaller, error) {
 	var tmID uint
 	if teamID != nil {
 		tmID = *teamID
 	}
 
+	const getInstallerIDStmt = `
+SELECT
+	id,
+	team_id,
+	title_id,
+	storage_id,
+	filename,
+	version,
+	install_script_content_id,
+	pre_install_query,
+	post_install_script_content_id,
+	uploaded_at
+FROM
+	software_installers
+WHERE
+	title_id = ? AND global_or_team_id = ?`
+
+	var installer fleet.SoftwareInstaller
+	err := sqlx.GetContext(ctx, ds.reader(ctx), &installer, getInstallerIDStmt, softwareTitleID, tmID)
+	if err != nil {
+		if err == sql.ErrNoRows {
+			return nil, notFound("SoftwareInstaller")
+		}
+
+		return nil, ctxerr.Wrap(ctx, err, "finding software installer by title")
+	}
+
+	return &installer, nil
+}
+
+func (ds *Datastore) InsertSoftwareInstallRequest(ctx context.Context, hostID uint, softwareInstallerID uint) error {
 	const (
 		insertStmt = `
 		  INSERT INTO host_software_installs
@@ -206,8 +237,6 @@ func (ds *Datastore) InsertSoftwareInstallRequest(ctx context.Context, hostID ui
 		    (?, ?, ?, ?)
 		    `
 
-		getInstallerIDStmt = `SELECT id FROM software_installers WHERE title_id = ? AND global_or_team_id = ?`
-
 		hostExistsStmt = `SELECT 1 FROM hosts WHERE id = ?`
 	)
 
@@ -219,17 +248,7 @@ func (ds *Datastore) InsertSoftwareInstallRequest(ctx context.Context, hostID ui
 			return notFound("Host").WithID(hostID)
 		}
 
-		return ctxerr.Wrap(ctx, err, "inserting new install software request")
-	}
-
-	var installerID uint
-	err = sqlx.GetContext(ctx, ds.reader(ctx), &installerID, getInstallerIDStmt, softwareTitleID, tmID)
-	if err != nil {
-		if err == sql.ErrNoRows {
-			return notFound("SoftwareInstaller")
-		}
-
-		return ctxerr.Wrap(ctx, err, "inserting new install software request")
+		return ctxerr.Wrap(ctx, err, "checking if host exists")
 	}
 
 	var userID *uint
@@ -239,7 +258,7 @@ func (ds *Datastore) InsertSoftwareInstallRequest(ctx context.Context, hostID ui
 	_, err = ds.writer(ctx).ExecContext(ctx, insertStmt,
 		uuid.NewString(),
 		hostID,
-		installerID,
+		softwareInstallerID,
 		userID,
 	)
 

server/datastore/mysql/software_installers_test.go

@@ -22,8 +22,8 @@ func TestSoftwareInstallers(t *testing.T) {
 		name string
 		fn   func(t *testing.T, ds *Datastore)
 	}{
+		{"SoftwareInstallRequests", testSoftwareInstallRequests},
 		{"SoftwareInstallerDetails", testListSoftwareInstallerDetails},
-		{"InsertSoftwareInstallRequest", testInsertSoftwareInstallRequest},
 		{"GetSoftwareInstallResults", testGetSoftwareInstallResult},
 	}
 
@@ -171,7 +171,7 @@ func insertSoftwareInstaller(
 	return res, nil
 }
 
-func testInsertSoftwareInstallRequest(t *testing.T, ds *Datastore) {
+func testSoftwareInstallRequests(t *testing.T, ds *Datastore) {
 	ctx := context.Background()
 
 	// create a team
@@ -185,23 +185,31 @@ func testInsertSoftwareInstallRequest(t *testing.T, ds *Datastore) {
 
 	for tc, teamID := range cases {
 		t.Run(tc, func(t *testing.T) {
-			// non-existent installer and host does the installer check first
-			err := ds.InsertSoftwareInstallRequest(ctx, 1, 1, teamID)
+			// non-existent installer
+			si, err := ds.GetSoftwareInstallerForTitle(ctx, 1, teamID)
 			var nfe fleet.NotFoundError
 			require.ErrorAs(t, err, &nfe)
+			require.Nil(t, si)
 
-			// non-existent host
 			installerID, err := ds.MatchOrCreateSoftwareInstaller(ctx, &fleet.UploadSoftwareInstallerPayload{
 				Title:         "foo",
 				Source:        "bar",
 				InstallScript: "echo",
 				TeamID:        teamID,
+				Filename:      "foo.pkg",
 			})
 			require.NoError(t, err)
+
 			installerMeta, err := ds.GetSoftwareInstallerMetadata(ctx, installerID)
 			require.NoError(t, err)
 
-			err = ds.InsertSoftwareInstallRequest(ctx, 12, installerMeta.TitleID, teamID)
+			si, err = ds.GetSoftwareInstallerForTitle(ctx, installerMeta.TitleID, teamID)
+			require.NoError(t, err)
+			require.NotNil(t, si)
+			require.Equal(t, "foo.pkg", si.Name)
+
+			// non-existent host
+			err = ds.InsertSoftwareInstallRequest(ctx, 12, si.InstallerID)
 			require.ErrorAs(t, err, &nfe)
 
 			// successful insert
@@ -214,7 +222,7 @@ func testInsertSoftwareInstallRequest(t *testing.T, ds *Datastore) {
 				TeamID:        teamID,
 			})
 			require.NoError(t, err)
-			err = ds.InsertSoftwareInstallRequest(ctx, host.ID, installerMeta.TitleID, teamID)
+			err = ds.InsertSoftwareInstallRequest(ctx, host.ID, si.InstallerID)
 			require.NoError(t, err)
 		})
 	}

server/fleet/datastore.go

@@ -491,7 +491,10 @@ type Datastore interface {
 	SoftwareTitleByID(ctx context.Context, id uint, teamID *uint, tmFilter TeamFilter) (*SoftwareTitle, error)
 
 	// InsertSoftwareInstallRequest tracks a new request to install the provided software installer in the host
-	InsertSoftwareInstallRequest(ctx context.Context, hostID uint, softwareInstallerID uint, teamID *uint) error
+	InsertSoftwareInstallRequest(ctx context.Context, hostID uint, softwareTitleID uint) error
+
+	// GetSoftwareInstallerForTitle TODO
+	GetSoftwareInstallerForTitle(ctx context.Context, softwareTitleID uint, teamID *uint) (*SoftwareInstaller, error)
 
 	///////////////////////////////////////////////////////////////////////////////
 	// SoftwareStore

server/mock/datastore_mock.go

@@ -365,7 +365,9 @@ type ListSoftwareTitlesFunc func(ctx context.Context, opt fleet.SoftwareTitleLis
 
 type SoftwareTitleByIDFunc func(ctx context.Context, id uint, teamID *uint, tmFilter fleet.TeamFilter) (*fleet.SoftwareTitle, error)
 
-type InsertSoftwareInstallRequestFunc func(ctx context.Context, hostID uint, softwareInstallerID uint, teamID *uint) error
+type InsertSoftwareInstallRequestFunc func(ctx context.Context, hostID uint, softwareTitleID uint) error
+
+type GetSoftwareInstallerForTitleFunc func(ctx context.Context, softwareTitleID uint, teamID *uint) (*fleet.SoftwareInstaller, error)
 
 type ListSoftwareForVulnDetectionFunc func(ctx context.Context, hostID uint) ([]fleet.Software, error)
 
@@ -1460,6 +1462,9 @@ type DataStore struct {
 	InsertSoftwareInstallRequestFunc        InsertSoftwareInstallRequestFunc
 	InsertSoftwareInstallRequestFuncInvoked bool
 
+	GetSoftwareInstallerForTitleFunc        GetSoftwareInstallerForTitleFunc
+	GetSoftwareInstallerForTitleFuncInvoked bool
+
 	ListSoftwareForVulnDetectionFunc        ListSoftwareForVulnDetectionFunc
 	ListSoftwareForVulnDetectionFuncInvoked bool
 
@@ -3529,11 +3534,18 @@ func (s *DataStore) SoftwareTitleByID(ctx context.Context, id uint, teamID *uint
 	return s.SoftwareTitleByIDFunc(ctx, id, teamID, tmFilter)
 }
 
-func (s *DataStore) InsertSoftwareInstallRequest(ctx context.Context, hostID uint, softwareInstallerID uint, teamID *uint) error {
+func (s *DataStore) InsertSoftwareInstallRequest(ctx context.Context, hostID uint, softwareTitleID uint) error {
 	s.mu.Lock()
 	s.InsertSoftwareInstallRequestFuncInvoked = true
 	s.mu.Unlock()
-	return s.InsertSoftwareInstallRequestFunc(ctx, hostID, softwareInstallerID, teamID)
+	return s.InsertSoftwareInstallRequestFunc(ctx, hostID, softwareTitleID)
+}
+
+func (s *DataStore) GetSoftwareInstallerForTitle(ctx context.Context, softwareTitleID uint, teamID *uint) (*fleet.SoftwareInstaller, error) {
+	s.mu.Lock()
+	s.GetSoftwareInstallerForTitleFuncInvoked = true
+	s.mu.Unlock()
+	return s.GetSoftwareInstallerForTitleFunc(ctx, softwareTitleID, teamID)
 }
 
 func (s *DataStore) ListSoftwareForVulnDetection(ctx context.Context, hostID uint) ([]fleet.Software, error) {

server/service/integration_core_test.go

@@ -11141,7 +11141,7 @@ func (s *integrationTestSuite) TestListHostUpcomingActivities() {
 	require.NoError(t, err)
 	s1Meta, err := s.ds.GetSoftwareInstallerMetadata(ctx, sw1)
 	require.NoError(t, err)
-	err = s.ds.InsertSoftwareInstallRequest(ctx, host1.ID, s1Meta.TitleID, nil)
+	err = s.ds.InsertSoftwareInstallRequest(ctx, host1.ID, s1Meta.InstallerID)
 	require.NoError(t, err)
 	h1Foo := latestSoftwareInstallerUUID()
 

server/service/integration_mdm_test.go

@@ -6,6 +6,7 @@ import (
 	"crypto/x509"
 	"database/sql"
 	"encoding/base64"
+	"encoding/hex"
 	"encoding/json"
 	"encoding/xml"
 	"errors"
@@ -8637,6 +8638,96 @@ func (s *integrationMDMTestSuite) TestSoftwareInstallerUploadDownloadAndDelete()
 	})
 }
 
+func (s *integrationMDMTestSuite) TestSoftwareInstallerNewInstallRequestPlatformValidation() {
+	t := s.T()
+
+	hostsByPlatform := map[string]*fleet.Host{
+		"linux": nil, "darwin": nil, "windows": nil,
+	}
+
+	tm, err := s.ds.NewTeam(context.Background(), &fleet.Team{
+		Name:        t.Name(),
+		Description: "desc",
+	})
+	require.NoError(t, err)
+
+	for platform := range hostsByPlatform {
+		h, err := s.ds.NewHost(context.Background(), &fleet.Host{
+			DetailUpdatedAt: time.Now(),
+			LabelUpdatedAt:  time.Now(),
+			PolicyUpdatedAt: time.Now(),
+			SeenTime:        time.Now().Add(-1 * time.Minute),
+			OsqueryHostID:   ptr.String(t.Name() + uuid.New().String()),
+			NodeKey:         ptr.String(t.Name() + uuid.New().String()),
+			Hostname:        fmt.Sprintf("%sfoo.local", t.Name()),
+			Platform:        platform,
+		})
+		require.NoError(t, err)
+		setOrbitEnrollment(t, h, s.ds)
+
+		err = s.ds.AddHostsToTeam(context.Background(), &tm.ID, []uint{h.ID})
+		require.NoError(t, err)
+
+		hostsByPlatform[platform] = h
+	}
+
+	softwareTitles := map[string]uint{
+		"deb": 0, "msi": 0, "exe": 0, "pkg": 0,
+	}
+
+	for kind := range softwareTitles {
+		// TODO(roberto): we need real binaries for exe, msi and pkg to
+		// perform the API calls.
+		mysql.ExecAdhocSQL(t, s.ds, func(q sqlx.ExtContext) error {
+			ctx := context.Background()
+			installScript := fmt.Sprintf(`echo '%s'`, kind)
+			res, err := q.ExecContext(ctx, `INSERT INTO script_contents (md5_checksum, contents) VALUES (UNHEX(md5(?)), ?)`, installScript, installScript)
+			if err != nil {
+				return err
+			}
+			scriptContentID, _ := res.LastInsertId()
+
+			res, err = q.ExecContext(ctx, `INSERT INTO software_titles (name, source) VALUES ('foo', ?)`, kind)
+			if err != nil {
+				return err
+			}
+			titleID, _ := res.LastInsertId()
+			softwareTitles[kind] = uint(titleID)
+
+			_, err = q.ExecContext(ctx, `
+			INSERT INTO software_installers
+				(title_id, filename, version, install_script_content_id, storage_id, team_id, global_or_team_id, pre_install_query)
+			VALUES
+				(?, ?, ?, ?, unhex(?), ?, ?, ?)`,
+				titleID, fmt.Sprintf("installer.%s", kind), "v1.0.0", scriptContentID, hex.EncodeToString([]byte("test")), tm.ID, tm.ID, "foo")
+			return err
+		})
+	}
+
+	testCases := []struct {
+		platform            string
+		supportedInstallers []string
+	}{
+		{"windows", []string{"exe", "msi"}},
+		{"darwin", []string{"pkg"}},
+		{"linux", []string{"deb"}},
+	}
+
+	for _, tc := range testCases {
+		for platform, host := range hostsByPlatform {
+			for _, kind := range tc.supportedInstallers {
+				wantStatus := http.StatusAccepted
+				if tc.platform != platform {
+					wantStatus = http.StatusBadRequest
+				}
+
+				var resp installSoftwareResponse
+				s.DoJSON("POST", fmt.Sprintf("/api/v1/fleet/hosts/%d/software/install/%d", host.ID, softwareTitles[kind]), nil, wantStatus, &resp)
+			}
+		}
+	}
+}
+
 func (s *integrationMDMTestSuite) TestSoftwareInstallerNewInstallRequest() {
 	t := s.T()
 
@@ -8653,7 +8744,7 @@ func (s *integrationMDMTestSuite) TestSoftwareInstallerNewInstallRequest() {
 		OsqueryHostID:   ptr.String(t.Name() + uuid.New().String()),
 		NodeKey:         ptr.String(t.Name() + uuid.New().String()),
 		Hostname:        fmt.Sprintf("%sfoo.local", t.Name()),
-		Platform:        "darwin",
+		Platform:        "linux",
 	})
 	require.NoError(t, err)