Clear unsupported params for script packages (#34297)

Implements #33752. Clears unsupported parameters for `.sh` and `.ps1` installers, plus tests.
This commit is contained in:
Carlo 2025-10-15 20:43:58 -04:00 committed by GitHub
parent 32f2ddaa63
commit 50ff9249c4
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 206 additions and 58 deletions

View file

@ -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{}{

View file

@ -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

View file

@ -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)