Fix software-with-bundle-ID add when the same title with different/no bundle ID, add missing request timeout special case for edit package endpoint (#22413)

Same as #22412, for #21370, but against `main` rather than 4.57.0.

# Checklist for submitter

If some of the following don't apply, delete the relevant line.

<!-- Note that API documentation changes are now addressed by the
product design team. -->

- [x] Changes file added for user-visible changes in `changes/`,
`orbit/changes/` or `ee/fleetd-chrome/changes`.
See [Changes
files](https://github.com/fleetdm/fleet/blob/main/docs/Contributing/Committing-Changes.md#changes-files)
for more information.
- [x] Added/updated tests
- [x] Manual QA for all new/changed functionality

---------

Co-authored-by: Roberto Dip <rroperzh@gmail.com>

articles/deploy-software-packages.md

@@ -14,7 +14,7 @@ Fleet [v4.50.0](https://github.com/fleetdm/fleet/releases/tag/fleet-v4.50.0) int
 
 * An S3 bucket [configured](https://fleetdm.com/docs/configuration/fleet-server-configuration#s-3-software-installers-bucket) to store the installers.
 
-* Increase any load balancer timeouts to at least 5 minutes for the [Add software](https://fleetdm.com/docs/rest-api/rest-api#add-software) endpoint.
+* Increase any load balancer timeouts to at least 5 minutes for the [Add package](https://fleetdm.com/docs/rest-api/rest-api#add-package) and [Modify package](https://fleetdm.com/docs/rest-api/rest-api#modify-package) endpoints.
 
 ## Step-by-step instructions
 

changes/21370-bundle-id-quickfix

@@ -0,0 +1 @@
+- Fix "no rows" error when adding a software installer that matches an existing title's name and source but not its bundle ID

changes/software-edit-request-deadline

@@ -0,0 +1 @@
+* Ensure request timeouts for software installer edits are just as high as for initial software installer uploads

cmd/fleet/serve.go

@@ -1145,7 +1145,8 @@ the way that the Fleet server works.
 					}
 				}
 
-				if req.Method == http.MethodPost && strings.HasSuffix(req.URL.Path, "/fleet/software/package") {
+				if (req.Method == http.MethodPost && strings.HasSuffix(req.URL.Path, "/fleet/software/package")) ||
+					(req.Method == http.MethodPatch && strings.HasSuffix(req.URL.Path, "/package") && strings.Contains(req.URL.Path, "/fleet/software/titles/")) {
 					// when uploading a software installer, the file might be large so
 					// the read timeout (to read the full request body) must be extended.
 					rc := http.NewResponseController(rw)

server/datastore/mysql/software_installers.go

@@ -174,8 +174,9 @@ func (ds *Datastore) getOrGenerateSoftwareInstallerTitleID(ctx context.Context,
 	insertArgs := []any{payload.Title, payload.Source}
 
 	if payload.BundleIdentifier != "" {
-		selectStmt = `SELECT id FROM software_titles WHERE bundle_identifier = ?`
-		selectArgs = []any{payload.BundleIdentifier}
+		// match by bundle identifier first, or standard matching if we don't have a bundle identifier match
+		selectStmt = `SELECT id FROM software_titles WHERE bundle_identifier = ? OR (name = ? AND source = ? AND browser = '') ORDER BY bundle_identifier = ? DESC LIMIT 1`
+		selectArgs = []any{payload.BundleIdentifier, payload.Title, payload.Source, payload.BundleIdentifier}
 		insertStmt = `INSERT INTO software_titles (name, source, bundle_identifier, browser) VALUES (?, ?, ?, '')`
 		insertArgs = append(insertArgs, payload.BundleIdentifier)
 	}

server/datastore/mysql/software_installers_test.go

@@ -34,6 +34,7 @@ func TestSoftwareInstallers(t *testing.T) {
 		{"HasSelfServiceSoftwareInstallers", testHasSelfServiceSoftwareInstallers},
 		{"DeleteSoftwareInstallersAssignedToPolicy", testDeleteSoftwareInstallersAssignedToPolicy},
 		{"GetHostLastInstallData", testGetHostLastInstallData},
+		{"GetOrGenerateSoftwareInstallerTitleID", testGetOrGenerateSoftwareInstallerTitleID},
 	}
 
 	for _, c := range cases {
@@ -1098,3 +1099,86 @@ func testGetHostLastInstallData(t *testing.T, ds *Datastore) {
 	require.NoError(t, err)
 	require.Nil(t, host2LastInstall)
 }
+
+func testGetOrGenerateSoftwareInstallerTitleID(t *testing.T, ds *Datastore) {
+	ctx := context.Background()
+
+	host1 := test.NewHost(t, ds, "host1", "", "host1key", "host1uuid", time.Now())
+	host2 := test.NewHost(t, ds, "host2", "", "host2key", "host2uuid", time.Now())
+
+	software1 := []fleet.Software{
+		{Name: "Existing Title", Version: "0.0.1", Source: "apps", BundleIdentifier: "existing.title"},
+	}
+	software2 := []fleet.Software{
+		{Name: "Existing Title", Version: "v0.0.2", Source: "apps", BundleIdentifier: "existing.title"},
+		{Name: "Existing Title", Version: "0.0.3", Source: "apps", BundleIdentifier: "existing.title"},
+		{Name: "Existing Title Without Bundle", Version: "0.0.3", Source: "apps"},
+	}
+
+	_, err := ds.UpdateHostSoftware(ctx, host1.ID, software1)
+	require.NoError(t, err)
+	_, err = ds.UpdateHostSoftware(ctx, host2.ID, software2)
+	require.NoError(t, err)
+	require.NoError(t, ds.SyncHostsSoftware(ctx, time.Now()))
+	require.NoError(t, ds.ReconcileSoftwareTitles(ctx))
+	require.NoError(t, ds.SyncHostsSoftwareTitles(ctx, time.Now()))
+
+	tests := []struct {
+		name    string
+		payload *fleet.UploadSoftwareInstallerPayload
+	}{
+		{
+			name: "title that already exists, no bundle identifier in payload",
+			payload: &fleet.UploadSoftwareInstallerPayload{
+				Title:  "Existing Title",
+				Source: "apps",
+			},
+		},
+		{
+			name: "title that already exists, mismatched bundle identifier in payload",
+			payload: &fleet.UploadSoftwareInstallerPayload{
+				Title:            "Existing Title",
+				Source:           "apps",
+				BundleIdentifier: "com.existing.bundle",
+			},
+		},
+		{
+			name: "title that already exists but doesn't have a bundle identifier",
+			payload: &fleet.UploadSoftwareInstallerPayload{
+				Title:  "Existing Title Without Bundle",
+				Source: "apps",
+			},
+		},
+		{
+			name: "title that already exists, no bundle identifier in DB, bundle identifier in payload",
+			payload: &fleet.UploadSoftwareInstallerPayload{
+				Title:            "Existing Title Without Bundle",
+				Source:           "apps",
+				BundleIdentifier: "com.new.bundleid",
+			},
+		},
+		{
+			name: "title that doesn't exist, no bundle identifier in payload",
+			payload: &fleet.UploadSoftwareInstallerPayload{
+				Title:  "New Title",
+				Source: "some_source",
+			},
+		},
+		{
+			name: "title that doesn't exist, with bundle identifier in payload",
+			payload: &fleet.UploadSoftwareInstallerPayload{
+				Title:            "New Title With Bundle",
+				Source:           "some_source",
+				BundleIdentifier: "com.new.bundle",
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			id, err := ds.getOrGenerateSoftwareInstallerTitleID(ctx, tt.payload)
+			require.NoError(t, err)
+			require.NotEmpty(t, id)
+		})
+	}
+}

server/datastore/mysql/vpp.go

@@ -371,17 +371,26 @@ func (ds *Datastore) getOrInsertSoftwareTitleForVPPApp(ctx context.Context, tx s
 	insertArgs := []any{app.Name, source}
 
 	if app.BundleIdentifier != "" {
-		// NOTE: The index `idx_sw_titles` doesn't include the bundle
-		// identifier. It's possible for the select to return nothing
-		// but for the insert to fail if an app with the same name but
-		// no bundle identifier exists in the DB.
+		// match by bundle identifier first, or standard matching if we
+		// don't have a bundle identifier match
 		switch source {
 		case "ios_apps", "ipados_apps":
-			selectStmt = `SELECT id FROM software_titles WHERE bundle_identifier = ? AND source = ?`
-			selectArgs = []any{app.BundleIdentifier, source}
+			selectStmt = `
+				    SELECT id
+				    FROM software_titles
+				    WHERE (bundle_identifier = ? AND source = ?) OR (name = ? AND source = ? AND browser = '')
+				    ORDER BY bundle_identifier = ? DESC
+				    LIMIT 1`
+			selectArgs = []any{app.BundleIdentifier, source, app.Name, source, app.BundleIdentifier}
 		default:
-			selectStmt = `SELECT id FROM software_titles WHERE bundle_identifier = ? AND source NOT IN ('ios_apps', 'ipados_apps')`
-			selectArgs = []any{app.BundleIdentifier}
+			selectStmt = `
+				    SELECT id
+				    FROM software_titles
+				    WHERE (bundle_identifier = ? OR (name = ? AND browser = ''))
+				      AND source NOT IN ('ios_apps', 'ipados_apps')
+				    ORDER BY bundle_identifier = ? DESC
+				    LIMIT 1`
+			selectArgs = []any{app.BundleIdentifier, app.Name, app.BundleIdentifier}
 		}
 		insertStmt = `INSERT INTO software_titles (name, source, bundle_identifier, browser) VALUES (?, ?, ?, '')`
 		insertArgs = append(insertArgs, app.BundleIdentifier)

server/datastore/mysql/vpp_test.go

@@ -2,6 +2,7 @@ package mysql
 
 import (
 	"context"
+	"fmt"
 	"testing"
 	"time"
 
@@ -30,6 +31,7 @@ func TestVPP(t *testing.T) {
 		{"GetVPPAppByTeamAndTitleID", testGetVPPAppByTeamAndTitleID},
 		{"VPPTokensCRUD", testVPPTokensCRUD},
 		{"VPPTokenAppTeamAssociations", testVPPTokenAppTeamAssociations},
+		{"GetOrInsertSoftwareTitleForVPPApp", testGetOrInsertSoftwareTitleForVPPApp},
 	}
 
 	for _, c := range cases {
@@ -1201,3 +1203,97 @@ func testVPPTokenAppTeamAssociations(t *testing.T, ds *Datastore) {
 	_, err = ds.InsertVPPAppWithTeam(ctx, app1, &team2.ID)
 	assert.Error(t, err)
 }
+
+func testGetOrInsertSoftwareTitleForVPPApp(t *testing.T, ds *Datastore) {
+	ctx := context.Background()
+
+	host1 := test.NewHost(t, ds, "host1", "", "host1key", "host1uuid", time.Now())
+	host2 := test.NewHost(t, ds, "host2", "", "host2key", "host2uuid", time.Now())
+
+	software1 := []fleet.Software{
+		{Name: "Existing Title", Version: "0.0.1", Source: "apps", BundleIdentifier: "existing.title"},
+	}
+	software2 := []fleet.Software{
+		{Name: "Existing Title", Version: "v0.0.2", Source: "apps", BundleIdentifier: "existing.title"},
+		{Name: "Existing Title", Version: "0.0.3", Source: "apps", BundleIdentifier: "existing.title"},
+		{Name: "Existing Title Without Bundle", Version: "0.0.3", Source: "apps"},
+	}
+
+	_, err := ds.UpdateHostSoftware(ctx, host1.ID, software1)
+	require.NoError(t, err)
+	_, err = ds.UpdateHostSoftware(ctx, host2.ID, software2)
+	require.NoError(t, err)
+	require.NoError(t, ds.SyncHostsSoftware(ctx, time.Now()))
+	require.NoError(t, ds.ReconcileSoftwareTitles(ctx))
+	require.NoError(t, ds.SyncHostsSoftwareTitles(ctx, time.Now()))
+
+	tests := []struct {
+		name string
+		app  *fleet.VPPApp
+	}{
+		{
+			name: "title that already exists, no bundle identifier in payload",
+			app: &fleet.VPPApp{
+				Name:             "Existing Title",
+				LatestVersion:    "0.0.1",
+				BundleIdentifier: "",
+			},
+		},
+		{
+			name: "title that already exists, bundle identifier in payload",
+			app: &fleet.VPPApp{
+				Name:             "Existing Title",
+				LatestVersion:    "0.0.2",
+				BundleIdentifier: "existing.title",
+			},
+		},
+		{
+			name: "title that already exists but doesn't have a bundle identifier",
+			app: &fleet.VPPApp{
+				Name:             "Existing Title Without Bundle",
+				LatestVersion:    "0.0.3",
+				BundleIdentifier: "",
+			},
+		},
+		{
+			name: "title that already exists, no bundle identifier in DB, bundle identifier in payload",
+			app: &fleet.VPPApp{
+				Name:             "Existing Title Without Bundle",
+				LatestVersion:    "0.0.3",
+				BundleIdentifier: "new.bundle.id",
+			},
+		},
+		{
+			name: "title that doesn't exist, no bundle identifier in payload",
+			app: &fleet.VPPApp{
+				Name:             "New Title",
+				LatestVersion:    "0.1.0",
+				BundleIdentifier: "",
+			},
+		},
+		{
+			name: "title that doesn't exist, with bundle identifier in payload",
+			app: &fleet.VPPApp{
+				Name:             "New Title",
+				LatestVersion:    "0.1.0",
+				BundleIdentifier: "new.title.bundle",
+			},
+		},
+	}
+
+	for _, platform := range fleet.VPPAppsPlatforms {
+		for _, tt := range tests {
+			t.Run(fmt.Sprintf("%s_%v", tt.name, platform), func(t *testing.T) {
+				tt.app.Platform = platform
+				var id uint
+				err := ds.withTx(ctx, func(tx sqlx.ExtContext) error {
+					var err error
+					id, err = ds.getOrInsertSoftwareTitleForVPPApp(ctx, tx, tt.app)
+					return err
+				})
+				require.NoError(t, err)
+				require.NotEmpty(t, id)
+			})
+		}
+	}
+}

server/service/integration_enterprise_test.go

@@ -14366,3 +14366,41 @@ func (s *integrationEnterpriseTestSuite) TestPolicyAutomationsSoftwareInstallers
 	require.NoError(t, err)
 	require.Nil(t, hostVanillaOsquery5Team1LastInstall)
 }
+
+func (s *integrationEnterpriseTestSuite) TestSoftwareInstallersWithoutBundleIdentifier() {
+	t := s.T()
+	ctx := context.Background()
+
+	// Create a host without a team
+	host, 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()),
+		NodeKey:         ptr.String(t.Name()),
+		UUID:            uuid.New().String(),
+		Hostname:        fmt.Sprintf("%sfoo.local", t.Name()),
+		Platform:        "darwin",
+	})
+	require.NoError(t, err)
+
+	software := []fleet.Software{
+		{Name: "DummyApp.app", Version: "0.0.2", Source: "apps"},
+	}
+	// we must ingest the title with an empty bundle identifier for this
+	// test to be valid
+	require.Empty(t, software[0].BundleIdentifier)
+	_, err = s.ds.UpdateHostSoftware(ctx, host.ID, software)
+	require.NoError(t, err)
+	require.NoError(t, s.ds.SyncHostsSoftware(ctx, time.Now()))
+	require.NoError(t, s.ds.ReconcileSoftwareTitles(ctx))
+	require.NoError(t, s.ds.SyncHostsSoftwareTitles(ctx, time.Now()))
+
+	payload := &fleet.UploadSoftwareInstallerPayload{
+		InstallScript: "install",
+		Filename:      "dummy_installer.pkg",
+		Version:       "0.0.2",
+	}
+	s.uploadSoftwareInstaller(payload, http.StatusOK, "")
+}