Return 404 for unknown group in workload listing

FilterByGroup returns an empty slice (nil error) for a group that does
not exist, so a typo'd ?group= silently returned 200 with an empty list
instead of the documented 404. Check group existence explicitly via the
group manager before filtering, in both the upgrade-check and the
listWorkloads handlers, so the advertised 404 is real. Add a bulk
upgrade-check test covering the unknown-group path.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Author: JAORMX

pkg/api/v1/workloads.go

@@ -144,9 +144,18 @@ func (s *WorkloadRoutes) listWorkloads(w http.ResponseWriter, r *http.Request) e
 				http.StatusBadRequest,
 			)
 		}
+		// FilterByGroup silently returns an empty slice for an unknown group,
+		// so check existence explicitly to honor the documented 404.
+		exists, existsErr := s.groupManager.Exists(ctx, groupFilter)
+		if existsErr != nil {
+			return fmt.Errorf("failed to check group existence: %w", existsErr)
+		}
+		if !exists {
+			return fmt.Errorf("%w: %s", groups.ErrGroupNotFound, groupFilter)
+		}
 		workloadList, err = workloads.FilterByGroup(workloadList, groupFilter)
 		if err != nil {
-			return err // groups.ErrGroupNotFound already has 404 status code
+			return err
 		}
 	}
 

pkg/api/v1/workloads_upgrade.go

@@ -14,6 +14,7 @@ import (
 
 	"github.com/stacklok/toolhive-core/httperr"
 	groupval "github.com/stacklok/toolhive-core/validation/group"
+	"github.com/stacklok/toolhive/pkg/groups"
 	"github.com/stacklok/toolhive/pkg/registry"
 	"github.com/stacklok/toolhive/pkg/runner"
 	"github.com/stacklok/toolhive/pkg/state"
@@ -120,9 +121,18 @@ func (s *WorkloadRoutes) upgradeCheckBulk(w http.ResponseWriter, r *http.Request
 				http.StatusBadRequest,
 			)
 		}
+		// FilterByGroup silently returns an empty slice for an unknown group,
+		// so check existence explicitly to honor the documented 404.
+		exists, existsErr := s.groupManager.Exists(ctx, groupFilter)
+		if existsErr != nil {
+			return fmt.Errorf("failed to check group existence: %w", existsErr)
+		}
+		if !exists {
+			return fmt.Errorf("%w: %s", groups.ErrGroupNotFound, groupFilter)
+		}
 		workloadList, err = workloads.FilterByGroup(workloadList, groupFilter)
 		if err != nil {
-			return err // groups.ErrGroupNotFound already has 404 status code
+			return err
 		}
 	}
 

pkg/api/v1/workloads_upgrade_test.go

@@ -20,6 +20,7 @@ import (
 	apierrors "github.com/stacklok/toolhive/pkg/api/errors"
 	"github.com/stacklok/toolhive/pkg/container/runtime"
 	"github.com/stacklok/toolhive/pkg/core"
+	groupsmocks "github.com/stacklok/toolhive/pkg/groups/mocks"
 	"github.com/stacklok/toolhive/pkg/registry"
 	registrymocks "github.com/stacklok/toolhive/pkg/registry/mocks"
 	"github.com/stacklok/toolhive/pkg/runner"
@@ -225,11 +226,15 @@ func TestUpgradeCheckBulk(t *testing.T) {
 		}, nil)
 		provider.EXPECT().GetServer("io.github/fetch").Return(imageServer("ghcr.io/example/fetch:1.1.0"), nil)
 
+		gm := groupsmocks.NewMockManager(ctrl)
+		gm.EXPECT().Exists(gomock.Any(), "prod").Return(true, nil)
+
 		configs := map[string]*runner.RunConfig{
 			"fetch": {Name: "fetch", Image: "ghcr.io/example/fetch:1.0.0", RegistryServerName: "io.github/fetch"},
 			"time":  {Name: "time", Image: "ghcr.io/example/time:1.0.0", RegistryServerName: "io.github/time"},
 		}
 		routes := upgradeTestRoutes(wm, provider, configs)
+		routes.groupManager = gm
 
 		req := httptest.NewRequest("GET", "/upgrade-check?group=prod", nil)
 		w := httptest.NewRecorder()
@@ -244,6 +249,33 @@ func TestUpgradeCheckBulk(t *testing.T) {
 		assert.Equal(t, upgrade.StatusUpgradeAvailable, resp.Results[0].Status)
 	})
 
+	t.Run("unknown group returns 404", func(t *testing.T) {
+		t.Parallel()
+
+		ctrl := gomock.NewController(t)
+		defer ctrl.Finish()
+
+		wm := workloadsmocks.NewMockManager(ctrl)
+		provider := registrymocks.NewMockProvider(ctrl)
+
+		// The group does not exist, so no per-workload registry lookup happens.
+		wm.EXPECT().ListWorkloads(gomock.Any(), false).Return([]core.Workload{
+			{Name: "fetch", Group: "prod"},
+		}, nil)
+
+		gm := groupsmocks.NewMockManager(ctrl)
+		gm.EXPECT().Exists(gomock.Any(), "ghost").Return(false, nil)
+
+		routes := upgradeTestRoutes(wm, provider, map[string]*runner.RunConfig{})
+		routes.groupManager = gm
+
+		req := httptest.NewRequest("GET", "/upgrade-check?group=ghost", nil)
+		w := httptest.NewRecorder()
+		apierrors.ErrorHandler(routes.upgradeCheckBulk).ServeHTTP(w, req)
+
+		require.Equal(t, http.StatusNotFound, w.Code)
+	})
+
 	t.Run("invalid group name", func(t *testing.T) {
 		t.Parallel()