diff --git a/ee/server/service/software_installers.go b/ee/server/service/software_installers.go index 719e000e03..3c1a8f7019 100644 --- a/ee/server/service/software_installers.go +++ b/ee/server/service/software_installers.go @@ -64,6 +64,16 @@ func (svc *Service) UploadSoftwareInstaller(ctx context.Context, payload *fleet. } payload.UserID = vc.UserID() + // Determine extension early so we can clear unsupported params for script packages + ext := strings.ToLower(filepath.Ext(payload.Filename)) + ext = strings.TrimPrefix(ext, ".") + if fleet.IsScriptPackage(ext) { + // For script packages, clear unsupported params before any processing + payload.UninstallScript = "" + payload.PostInstallScript = "" + payload.PreInstallQuery = "" + } + // make sure all scripts use unix-style newlines to prevent errors when // running them, browsers use windows-style newlines, which breaks the // shebang when the file is directly executed. @@ -423,72 +433,93 @@ func (svc *Service) UpdateSoftwareInstaller(ctx context.Context, payload *fleet. payload.UpgradeCode = existingInstaller.UpgradeCode } + isScriptPackage := fleet.IsScriptPackage(existingInstaller.Extension) + // default pre-install query is blank, so blanking out the query doesn't have a semantic meaning we have to take care of - if payload.PreInstallQuery != nil && *payload.PreInstallQuery != existingInstaller.PreInstallQuery { - dirty["PreInstallQuery"] = true + if payload.PreInstallQuery != nil { + if isScriptPackage { + emptyQuery := "" + payload.PreInstallQuery = &emptyQuery + } else if *payload.PreInstallQuery != existingInstaller.PreInstallQuery { + dirty["PreInstallQuery"] = true + } } if payload.InstallScript != nil { - installScript := file.Dos2UnixNewlines(*payload.InstallScript) - if installScript == "" { - installScript = file.GetInstallScript(existingInstaller.Extension) - } - if installScript == "" { - return nil, &fleet.BadRequestError{ - Message: fmt.Sprintf("Couldn't edit. Install script is required for .%s packages.", strings.ToLower(existingInstaller.Extension)), + if isScriptPackage { + payload.InstallScript = nil + } else { + installScript := file.Dos2UnixNewlines(*payload.InstallScript) + if installScript == "" { + installScript = file.GetInstallScript(existingInstaller.Extension) + } + if installScript == "" { + return nil, &fleet.BadRequestError{ + Message: fmt.Sprintf("Couldn't edit. Install script is required for .%s packages.", strings.ToLower(existingInstaller.Extension)), + } } - } - if installScript != existingInstaller.InstallScript { - dirty["InstallScript"] = true + if installScript != existingInstaller.InstallScript { + dirty["InstallScript"] = true + } + payload.InstallScript = &installScript } - payload.InstallScript = &installScript } if payload.PostInstallScript != nil { - postInstallScript := file.Dos2UnixNewlines(*payload.PostInstallScript) - if postInstallScript != existingInstaller.PostInstallScript { - dirty["PostInstallScript"] = true + if isScriptPackage { + emptyScript := "" + payload.PostInstallScript = &emptyScript + } else { + postInstallScript := file.Dos2UnixNewlines(*payload.PostInstallScript) + if postInstallScript != existingInstaller.PostInstallScript { + dirty["PostInstallScript"] = true + } + payload.PostInstallScript = &postInstallScript } - payload.PostInstallScript = &postInstallScript } if payload.UninstallScript != nil { - uninstallScript := file.Dos2UnixNewlines(*payload.UninstallScript) - if uninstallScript == "" { // extension can't change on an edit so we can generate off of the existing file - uninstallScript = file.GetUninstallScript(existingInstaller.Extension) - if payload.UpgradeCode != "" { - uninstallScript = file.UninstallMsiWithUpgradeCodeScript + if isScriptPackage { + emptyScript := "" + payload.UninstallScript = &emptyScript + } else { + uninstallScript := file.Dos2UnixNewlines(*payload.UninstallScript) + if uninstallScript == "" { // extension can't change on an edit so we can generate off of the existing file + uninstallScript = file.GetUninstallScript(existingInstaller.Extension) + if payload.UpgradeCode != "" { + uninstallScript = file.UninstallMsiWithUpgradeCodeScript + } } - } - if uninstallScript == "" { - return nil, &fleet.BadRequestError{ - Message: fmt.Sprintf("Couldn't edit. Uninstall script is required for .%s packages.", strings.ToLower(existingInstaller.Extension)), + if uninstallScript == "" { + return nil, &fleet.BadRequestError{ + Message: fmt.Sprintf("Couldn't edit. Uninstall script is required for .%s packages.", strings.ToLower(existingInstaller.Extension)), + } } - } - payloadForUninstallScript := &fleet.UploadSoftwareInstallerPayload{ - Extension: existingInstaller.Extension, - UninstallScript: uninstallScript, - PackageIDs: existingInstaller.PackageIDs(), - UpgradeCode: existingInstaller.UpgradeCode, - } - if payloadForNewInstallerFile != nil { - payloadForUninstallScript.PackageIDs = payloadForNewInstallerFile.PackageIDs - payloadForUninstallScript.UpgradeCode = payloadForNewInstallerFile.UpgradeCode - } - - if err := preProcessUninstallScript(payloadForUninstallScript); err != nil { - return nil, &fleet.BadRequestError{ - Message: "Couldn't add. $UPGRADE_CODE variable was used but package does not have an UpgradeCode.", + payloadForUninstallScript := &fleet.UploadSoftwareInstallerPayload{ + Extension: existingInstaller.Extension, + UninstallScript: uninstallScript, + PackageIDs: existingInstaller.PackageIDs(), + UpgradeCode: existingInstaller.UpgradeCode, + } + if payloadForNewInstallerFile != nil { + payloadForUninstallScript.PackageIDs = payloadForNewInstallerFile.PackageIDs + payloadForUninstallScript.UpgradeCode = payloadForNewInstallerFile.UpgradeCode } - } - if payloadForUninstallScript.UninstallScript != existingInstaller.UninstallScript { - dirty["UninstallScript"] = true + if err := preProcessUninstallScript(payloadForUninstallScript); err != nil { + return nil, &fleet.BadRequestError{ + Message: "Couldn't add. $UPGRADE_CODE variable was used but package does not have an UpgradeCode.", + } + } + + if payloadForUninstallScript.UninstallScript != existingInstaller.UninstallScript { + dirty["UninstallScript"] = true + } + uninstallScript = payloadForUninstallScript.UninstallScript + payload.UninstallScript = &uninstallScript } - uninstallScript = payloadForUninstallScript.UninstallScript - payload.UninstallScript = &uninstallScript } fieldsShouldSideEffect := map[string]struct{}{ diff --git a/server/service/integration_enterprise_test.go b/server/service/integration_enterprise_test.go index 352cb3c69b..45dc5819cb 100644 --- a/server/service/integration_enterprise_test.go +++ b/server/service/integration_enterprise_test.go @@ -14810,6 +14810,116 @@ func (s *integrationEnterpriseTestSuite) TestEXEPackageUploads() { }, http.StatusOK, "") } +func (s *integrationEnterpriseTestSuite) TestScriptPackageUploads() { + t := s.T() + ctx := context.Background() + + team, err := s.ds.NewTeam(ctx, &fleet.Team{Name: t.Name() + "team1"}) + require.NoError(t, err) + + // Create temporary script files for testing + shContent := "#!/bin/bash\necho 'Installing...'\nexit 0\n" + ps1Content := "Write-Host 'Installing...'\nExit 0\n" + + shFile, err := fleet.NewTempFileReader(strings.NewReader(shContent), func() string { return t.TempDir() }) + require.NoError(t, err) + defer shFile.Close() + + ps1File, err := fleet.NewTempFileReader(strings.NewReader(ps1Content), func() string { return t.TempDir() }) + require.NoError(t, err) + defer ps1File.Close() + + // Test .sh file with automatic_install (should be rejected) + payload := &fleet.UploadSoftwareInstallerPayload{ + Filename: "install-app.sh", + TeamID: &team.ID, + AutomaticInstall: true, + InstallerFile: shFile, + } + s.uploadSoftwareInstaller(t, payload, http.StatusBadRequest, "Couldn't add. Fleet can't create a policy to detect existing installations for .sh packages.") + + // Rewind the file reader for reuse + err = shFile.Rewind() + require.NoError(t, err) + + // Test .ps1 file with automatic_install (should be rejected) + payload = &fleet.UploadSoftwareInstallerPayload{ + Filename: "install-app.ps1", + TeamID: &team.ID, + AutomaticInstall: true, + InstallerFile: ps1File, + } + s.uploadSoftwareInstaller(t, payload, http.StatusBadRequest, "Couldn't add. Fleet can't create a policy to detect existing installations for .ps1 packages.") + + // Rewind the file reader for reuse + err = shFile.Rewind() + require.NoError(t, err) + + // Test .sh file with unsupported params (should be ignored/cleared) + payload = &fleet.UploadSoftwareInstallerPayload{ + Filename: "install-app.sh", + TeamID: &team.ID, + UninstallScript: "echo 'uninstall'", + PostInstallScript: "echo 'post'", + PreInstallQuery: "SELECT 1;", + InstallerFile: shFile, + } + s.uploadSoftwareInstaller(t, payload, http.StatusOK, "") + + var titleID uint + mysql.ExecAdhocSQL(t, s.ds, func(q sqlx.ExtContext) error { + return sqlx.GetContext(context.Background(), q, &titleID, `SELECT title_id FROM software_installers WHERE global_or_team_id = ? AND filename = ?`, &team.ID, payload.Filename) + }) + require.NotZero(t, titleID) + + // Verify unsupported params were cleared (script contents should be empty) + var scriptContents struct { + UninstallScript string `db:"uninstall_script"` + PostInstallScript string `db:"post_install_script"` + PreInstallQuery string `db:"pre_install_query"` + } + mysql.ExecAdhocSQL(t, s.ds, func(q sqlx.ExtContext) error { + return sqlx.GetContext(context.Background(), q, &scriptContents, ` + SELECT + COALESCE(uninst.contents, '') AS uninstall_script, + COALESCE(postinst.contents, '') AS post_install_script, + pre_install_query + FROM software_installers si + LEFT JOIN script_contents uninst ON uninst.id = si.uninstall_script_content_id + LEFT JOIN script_contents postinst ON postinst.id = si.post_install_script_content_id + WHERE si.title_id = ?`, titleID) + }) + require.Empty(t, scriptContents.UninstallScript, "uninstall_script should be empty") + require.Empty(t, scriptContents.PostInstallScript, "post_install_script should be empty") + require.Empty(t, scriptContents.PreInstallQuery, "pre_install_query should be empty") + + // Test editing script package with unsupported params (should be ignored) + s.updateSoftwareInstaller(t, &fleet.UpdateSoftwareInstallerPayload{ + Filename: "install-app.sh", + UninstallScript: ptr.String("should be cleared"), + PostInstallScript: ptr.String("should be cleared"), + PreInstallQuery: ptr.String("should be cleared"), + TitleID: titleID, + TeamID: &team.ID, + }, http.StatusOK, "") + + // Verify unsupported params are still cleared after update + mysql.ExecAdhocSQL(t, s.ds, func(q sqlx.ExtContext) error { + return sqlx.GetContext(context.Background(), q, &scriptContents, ` + SELECT + COALESCE(uninst.contents, '') AS uninstall_script, + COALESCE(postinst.contents, '') AS post_install_script, + pre_install_query + FROM software_installers si + LEFT JOIN script_contents uninst ON uninst.id = si.uninstall_script_content_id + LEFT JOIN script_contents postinst ON postinst.id = si.post_install_script_content_id + WHERE si.title_id = ?`, titleID) + }) + require.Empty(t, scriptContents.UninstallScript, "uninstall_script should still be empty") + require.Empty(t, scriptContents.PostInstallScript, "post_install_script should still be empty") + require.Empty(t, scriptContents.PreInstallQuery, "pre_install_query should still be empty") +} + // 1. host reports software // 2. reconciler runs, creates title // 3. installer is uploaded, matches existing software title diff --git a/server/service/testing_client.go b/server/service/testing_client.go index c4ce166538..68556a60a6 100644 --- a/server/service/testing_client.go +++ b/server/service/testing_client.go @@ -671,19 +671,26 @@ func (ts *withServer) uploadSoftwareInstallerWithErrorNameReason( ) { t.Helper() - tfr, err := fleet.NewKeepFileReader(filepath.Join("testdata", "software-installers", payload.Filename)) - // Try the test installers in the pkg/file testdata (to reduce clutter/copies). - if errors.Is(err, os.ErrNotExist) { - var err2 error - tfr, err2 = fleet.NewKeepFileReader(filepath.Join("..", "..", "pkg", "file", "testdata", "software-installers", payload.Filename)) - if err2 == nil { - err = nil + // Determine which file to use: either provided by test or opened from testdata + var installerFile io.Reader + if payload.InstallerFile == nil { + // Open file from testdata and close it when done + tfr, err := fleet.NewKeepFileReader(filepath.Join("testdata", "software-installers", payload.Filename)) + // Try the test installers in the pkg/file testdata (to reduce clutter/copies). + if errors.Is(err, os.ErrNotExist) { + var err2 error + tfr, err2 = fleet.NewKeepFileReader(filepath.Join("..", "..", "pkg", "file", "testdata", "software-installers", payload.Filename)) + if err2 == nil { + err = nil + } } + require.NoError(t, err) + defer tfr.Close() + installerFile = tfr + } else { + // Use the file provided by the test + installerFile = payload.InstallerFile } - require.NoError(t, err) - defer tfr.Close() - - payload.InstallerFile = tfr var b bytes.Buffer w := multipart.NewWriter(&b) @@ -691,7 +698,7 @@ func (ts *withServer) uploadSoftwareInstallerWithErrorNameReason( // add the software field fw, err := w.CreateFormFile("software", payload.Filename) require.NoError(t, err) - n, err := io.Copy(fw, payload.InstallerFile) + n, err := io.Copy(fw, installerFile) require.NoError(t, err) require.NotZero(t, n)