Authorize packs before returning them in query responses (#50148)

- [X] Changes file added for user-visible changes in `changes/`,
`orbit/changes/` or `ee/fleetd-chrome/changes`.

## Testing

- [X] Added/updated automated tests
- [X] QA'd all new/changed functionality manually

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **Bug Fixes**
* Query responses now include pack details only when the requester has
permission to view them.
* Prevented pack metadata from being disclosed across fleets when query
names overlap.
* Corrected target selection labels and empty-state messaging for
fleet-based targets.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

changes/16773-query-pack-metadata-leak

@@ -0,0 +1 @@
+- Fixed query (report) responses so that pack metadata (ID, name, description) is only included when the requesting user is authorized to read packs, preventing cross-fleet pack metadata disclosure via query name collisions.

frontend/components/forms/fields/SelectTargetsDropdown/SelectTargetsMenu/SelectTargetsMenu.jsx

@@ -34,7 +34,12 @@ const SelectTargetsMenuWrapper = (
     const renderTargets = (targetType) => {
       const targets = filter(options, targetFilter(targetType));
       const targetsOutput = [];
-      const targetTitle = targetType === "all" ? "all hosts" : targetType;
+      let targetTitle = targetType;
+      if (targetType === "all") {
+        targetTitle = "all hosts";
+      } else if (targetType === "teams") {
+        targetTitle = "fleets";
+      }
 
       targetsOutput.push(
         <p className={`${baseClass}__type`} key={`type-${targetType}-key`}>
@@ -52,7 +57,7 @@ const SelectTargetsMenuWrapper = (
             className={`${baseClass}__not-found`}
             key={`${targetType}-notfound`}
           >
-            Unable to find any matching {targetType}.
+            Unable to find any matching {targetTitle}.
           </span>
         );
 
@@ -108,7 +113,7 @@ const SelectTargetsMenuWrapper = (
     const renderTargetGroups = (
       <>
         {renderTargets("all")}
-        {isPremiumTier && renderTargets("fleets")}
+        {isPremiumTier && renderTargets("teams")}
         {renderTargets("labels")}
         {renderTargets("hosts")}
       </>

server/service/queries.go

@@ -38,9 +38,29 @@ func (svc *Service) GetQuery(ctx context.Context, id uint) (*fleet.Query, error)
 	if err := svc.authz.Authorize(ctx, query, fleet.ActionRead); err != nil {
 		return nil, err
 	}
+	svc.filterQueryPacksForUser(ctx, query)
 	return query, nil
 }
 
+// filterQueryPacksForUser removes from the given queries the packs that the
+// requesting user is not authorized to read. Packs are associated to queries
+// by name (see loadPacksForQueries), so a query's Packs field may include
+// packs of same-named queries scoped to teams the user has no access to.
+func (svc *Service) filterQueryPacksForUser(ctx context.Context, queries ...*fleet.Query) {
+	for _, query := range queries {
+		if len(query.Packs) == 0 {
+			continue
+		}
+		authorizedPacks := make([]fleet.Pack, 0, len(query.Packs))
+		for _, pack := range query.Packs {
+			if err := svc.authz.Authorize(ctx, &pack, fleet.ActionRead); err == nil {
+				authorizedPacks = append(authorizedPacks, pack)
+			}
+		}
+		query.Packs = authorizedPacks
+	}
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 // List Queries
 ////////////////////////////////////////////////////////////////////////////////
@@ -116,6 +136,8 @@ func (svc *Service) ListQueries(ctx context.Context, opt fleet.ListOptions, team
 		return nil, 0, 0, nil, err
 	}
 
+	svc.filterQueryPacksForUser(ctx, queries...)
+
 	return queries, count, inheritedCount, meta, nil
 }
 
@@ -476,6 +498,7 @@ func (svc *Service) ModifyQuery(ctx context.Context, id uint, p fleet.QueryPaylo
 		return nil, ctxerr.Wrap(ctx, err, "create activity for query modification")
 	}
 
+	svc.filterQueryPacksForUser(ctx, query)
 	return query, nil
 }
 

server/service/queries_test.go

@@ -851,6 +851,70 @@ func TestQueryAuth(t *testing.T) {
 	}
 }
 
+func TestQueryResponsesFilterUnauthorizedPacks(t *testing.T) {
+	ds := new(mock.Store)
+	svc, ctx := newTestService(t, ds, nil, nil)
+
+	teamID := uint(1)
+	// Simulates a pack scoped to another team that got associated to this
+	// query via the name-based join in loadPacksForQueries.
+	otherTeamPack := fleet.Pack{ID: 123, Name: "other team pack", Description: "secret"}
+	teamQuery := fleet.Query{
+		ID:     88,
+		Name:   "shared name",
+		TeamID: &teamID,
+	}
+
+	ds.QueryFunc = func(ctx context.Context, id uint) (*fleet.Query, error) {
+		q := teamQuery
+		q.Packs = []fleet.Pack{otherTeamPack}
+		return &q, nil
+	}
+	ds.ListQueriesFunc = func(ctx context.Context, opts fleet.ListQueryOptions) ([]*fleet.Query, int, int, *fleet.PaginationMetadata, error) {
+		q := teamQuery
+		q.Packs = []fleet.Pack{otherTeamPack}
+		return []*fleet.Query{&q}, 1, 0, nil, nil
+	}
+
+	// A team observer can read the team query but is not authorized to read
+	// packs, so pack metadata must be filtered out of the response.
+	teamObserver := &fleet.User{
+		ID: 44,
+		Teams: []fleet.UserTeam{
+			{
+				Team: fleet.Team{ID: teamID},
+				Role: fleet.RoleObserver,
+			},
+		},
+	}
+	observerCtx := viewer.NewContext(ctx, viewer.Viewer{User: teamObserver})
+
+	query, err := svc.GetQuery(observerCtx, teamQuery.ID)
+	require.NoError(t, err)
+	require.Empty(t, query.Packs)
+
+	queries, _, _, _, err := svc.ListQueries(observerCtx, fleet.ListOptions{}, &teamID, nil, false, nil)
+	require.NoError(t, err)
+	require.Len(t, queries, 1)
+	require.Empty(t, queries[0].Packs)
+
+	// A global admin is authorized to read packs, so pack metadata is kept.
+	globalAdmin := &fleet.User{
+		ID:         1,
+		GlobalRole: new(fleet.RoleAdmin),
+	}
+	adminCtx := viewer.NewContext(ctx, viewer.Viewer{User: globalAdmin})
+
+	query, err = svc.GetQuery(adminCtx, teamQuery.ID)
+	require.NoError(t, err)
+	require.Equal(t, []fleet.Pack{otherTeamPack}, query.Packs)
+
+	queries, _, _, _, err = svc.ListQueries(adminCtx, fleet.ListOptions{}, &teamID, nil, false, nil)
+	require.NoError(t, err)
+	require.Len(t, queries, 1)
+	require.Equal(t, []fleet.Pack{otherTeamPack}, queries[0].Packs)
+}
+
 func TestQueryReportIsClipped(t *testing.T) {
 	ds := new(mock.Store)
 	svc, ctx := newTestService(t, ds, nil, nil)