From 81f2edec6509ffae4da64037270731902cf2f805 Mon Sep 17 00:00:00 2001 From: Sharon Katz <121527325+sharon-fdm@users.noreply.github.com> Date: Tue, 23 Jun 2026 14:28:15 -0400 Subject: [PATCH] Improve fleet scope validation for software title lookups (#48034) ## Summary Ensures that `SoftwareTitleByID` validates fleet scope for all non-nil `team_id` values, including zero. Previously the scope check was only applied when `team_id > 0`. ## Reproduction Added a unit test (`TestSoftwareTitleByIDTeamIDZero`) that sets up a fleet-scoped user on fleet 1, then calls `SoftwareTitleByID` with `team_id=0`. Before this change, the call succeeded. After, it correctly returns 403. Also confirmed that a global admin calling with `team_id=0` still succeeds, and that all existing `TestServiceSoftwareTitlesAuth` subtests continue to pass. # Checklist for submitter If some of the following don't apply, delete the relevant line. - [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/guides/committing-changes.md#changes-files) for more information. - [x] Input data is properly validated, `SELECT *` is avoided, SQL injection is prevented (using placeholders for values in statements), JS inline code is prevented especially for url redirects, and untrusted data interpolated into shell scripts/commands is validated against shell metacharacters. ## Testing - [x] Added/updated automated tests - [x] QA'd all new/changed functionality manually ## Summary by CodeRabbit * **Improvements** * Enhanced fleet scope validation for software title lookups, including correct handling when a team scope value is set to `0`. * **Tests** * Added unit test coverage for software title retrieval authorization behavior when the team scope value is `0`. --- changes/software-title-scope-validation | 1 + server/service/software_titles.go | 18 +++++++------ server/service/software_titles_test.go | 35 +++++++++++++++++++++++++ 3 files changed, 46 insertions(+), 8 deletions(-) create mode 100644 changes/software-title-scope-validation diff --git a/changes/software-title-scope-validation b/changes/software-title-scope-validation new file mode 100644 index 0000000000..6c7680301b --- /dev/null +++ b/changes/software-title-scope-validation @@ -0,0 +1 @@ +- Improved fleet scope validation for software title lookups. diff --git a/server/service/software_titles.go b/server/service/software_titles.go index 562223ca3c..2167c6088f 100644 --- a/server/service/software_titles.go +++ b/server/service/software_titles.go @@ -143,17 +143,19 @@ func (svc *Service) SoftwareTitleByID(ctx context.Context, id uint, teamID *uint return nil, err } - if teamID != nil && *teamID != 0 { - // This auth check ensures we return 403 if the user doesn't have access to the team + if teamID != nil { + // Verify the caller has permission for the requested scope (team or global). if err := svc.authz.Authorize(ctx, &fleet.AuthzSoftwareInventory{TeamID: teamID}, fleet.ActionRead); err != nil { return nil, err } - exists, err := svc.ds.TeamExists(ctx, *teamID) - if err != nil { - return nil, ctxerr.Wrap(ctx, err, "checking if team exists") - } else if !exists { - return nil, fleet.NewInvalidArgumentError("team_id", fmt.Sprintf("fleet %d does not exist", *teamID)). - WithStatus(http.StatusNotFound) + if *teamID != 0 { + exists, err := svc.ds.TeamExists(ctx, *teamID) + if err != nil { + return nil, ctxerr.Wrap(ctx, err, "checking if team exists") + } else if !exists { + return nil, fleet.NewInvalidArgumentError("team_id/fleet_id", fmt.Sprintf("fleet %d does not exist", *teamID)). + WithStatus(http.StatusNotFound) + } } } diff --git a/server/service/software_titles_test.go b/server/service/software_titles_test.go index 604baa18fc..f1381c2512 100644 --- a/server/service/software_titles_test.go +++ b/server/service/software_titles_test.go @@ -272,3 +272,38 @@ func TestSoftwareNameUpdate(t *testing.T) { require.NoError(t, err) require.True(t, ds.UpdateSoftwareTitleNameFuncInvoked) } + +func TestSoftwareTitleByIDTeamIDZero(t *testing.T) { + ds := new(mock.Store) + ds.SoftwareTitleByIDFunc = func(ctx context.Context, id uint, teamID *uint, tmFilter fleet.TeamFilter) (*fleet.SoftwareTitle, error) { + return &fleet.SoftwareTitle{BundleIdentifier: new("com.example.app")}, nil + } + ds.TeamExistsFunc = func(ctx context.Context, teamID uint) (bool, error) { return true, nil } + + svc, ctx := newTestService(t, ds, nil, nil) + + teamIDZero := new(uint) // *uint pointing to 0 + + // Team-scoped user on team 1 should not be able to access software with team_id=0 + teamUser := &fleet.User{ + ID: 1, + Teams: []fleet.UserTeam{{ + Team: fleet.Team{ID: 1}, + Role: fleet.RoleAdmin, + }}, + } + ctx = viewer.NewContext(ctx, viewer.Viewer{User: teamUser}) + + _, err := svc.SoftwareTitleByID(ctx, 1, teamIDZero) + checkAuthErr(t, true, err) + + // Global admin should still be able to access software with team_id=0 + globalAdmin := &fleet.User{ + ID: 2, + GlobalRole: new(fleet.RoleAdmin), + } + adminCtx := viewer.NewContext(ctx, viewer.Viewer{User: globalAdmin}) + + _, err = svc.SoftwareTitleByID(adminCtx, 1, teamIDZero) + checkAuthErr(t, false, err) +}