Stop the delete host endpoint from revealing out-of-fleet host existence (#49645)

<!-- Add the related story/sub-task/bug number, like Resolves #123, or
remove if NA -->
**Related issue:** Resolves: N/A

# Checklist for submitter

- [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.

## 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**
* Updated host deletion behavior to return consistent “not found”
responses when the host doesn’t exist or isn’t visible to the requester.
* Prevented out-of-scope delete attempts from disclosing whether the
target host exists (now returns “not found” instead of “forbidden”).
* Preserved “forbidden” errors when the host is visible but the
requester lacks delete permission.
* **Tests**
* Added/updated authorization and deletion coverage to verify the new
response-masking behavior.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

changes/fix-delete-host-response-mismatch

@@ -0,0 +1 @@
+* Fixed the delete host endpoint returning inconsistent responses for a host outside the requester's fleet versus one that doesn't exist.

server/authz/authz.go

@@ -15,6 +15,7 @@ import (
 	"fmt"
 
 	authz_ctx "github.com/fleetdm/fleet/v4/server/contexts/authz"
+	"github.com/fleetdm/fleet/v4/server/contexts/ctxerr"
 	"github.com/fleetdm/fleet/v4/server/contexts/viewer"
 	"github.com/fleetdm/fleet/v4/server/fleet"
 	platform_authz "github.com/fleetdm/fleet/v4/server/platform/authz"
@@ -132,6 +133,33 @@ func (a *Authorizer) Authorize(ctx context.Context, object, action interface{})
 	return nil
 }
 
+// AuthorizeOrNotFound authorizes writeAction on writeObject. If that fails,
+// it also checks fleet.ActionRead on readObject; if the caller can't even
+// read it, notFoundErr is returned instead of the write failure, so a
+// resource entirely outside the caller's visibility is indistinguishable
+// from one that doesn't exist. If the caller CAN read it, or notFoundErr is
+// nil, the original write-authorization failure is returned unchanged: in
+// the first case no new information is disclosed by it (the caller already
+// knows the resource exists); in the second, masking would incorrectly
+// return nil (success) for a caller who was never authorized.
+func (a *Authorizer) AuthorizeOrNotFound(ctx context.Context, writeObject, writeAction, readObject any, notFoundErr error) error {
+	writeErr := a.Authorize(ctx, writeObject, writeAction)
+	if writeErr == nil {
+		return nil
+	}
+
+	if readErr := a.Authorize(ctx, readObject, fleet.ActionRead); readErr == nil || notFoundErr == nil {
+		return writeErr
+	}
+
+	// The caller can't read this resource either: report notFoundErr instead
+	// of writeErr, so its existence isn't disclosed. Still record the real
+	// cause for observability, so a systemic authz/policy failure isn't
+	// silently reported as "not found" for every caller.
+	ctxerr.Handle(ctx, writeErr)
+	return notFoundErr
+}
+
 // ExtraAuthzer is the interface to implement extra fields for the policy.
 type ExtraAuthzer interface {
 	// ExtraAuthz returns the extra key/value pairs for the type.

server/authz/authz_test.go

@@ -0,0 +1,54 @@
+package authz
+
+import (
+	"errors"
+	"testing"
+
+	"github.com/fleetdm/fleet/v4/server/fleet"
+	"github.com/fleetdm/fleet/v4/server/test"
+	"github.com/stretchr/testify/require"
+)
+
+func TestAuthorizeOrNotFound(t *testing.T) {
+	notFoundErr := errors.New("not found sentinel")
+	teamHost := &fleet.Host{TeamID: new(uint(1))}
+
+	t.Run("write allowed", func(t *testing.T) {
+		ctx := test.UserContext(t.Context(), &fleet.User{Teams: []fleet.UserTeam{{Team: fleet.Team{ID: 1}, Role: fleet.RoleAdmin}}})
+		err := auth.AuthorizeOrNotFound(ctx, teamHost, fleet.ActionWrite, teamHost, notFoundErr)
+		require.NoError(t, err)
+	})
+
+	t.Run("write denied but read allowed returns the write error, not masked", func(t *testing.T) {
+		// A team observer can read the host but can't write it: this is not
+		// an existence oracle (the caller already knows the host exists), so
+		// the real Forbidden should surface, not notFoundErr.
+		ctx := test.UserContext(t.Context(), &fleet.User{Teams: []fleet.UserTeam{{Team: fleet.Team{ID: 1}, Role: fleet.RoleObserver}}})
+		err := auth.AuthorizeOrNotFound(ctx, teamHost, fleet.ActionWrite, teamHost, notFoundErr)
+		require.Error(t, err)
+		require.NotErrorIs(t, err, notFoundErr)
+		var forbidden *Forbidden
+		require.ErrorAs(t, err, &forbidden)
+	})
+
+	t.Run("write denied and read denied masks as notFoundErr", func(t *testing.T) {
+		// A caller with no relationship to the host's team can't read or
+		// write it: masking as notFoundErr prevents them from learning the
+		// host exists on some other team via a distinguishable Forbidden.
+		ctx := test.UserContext(t.Context(), &fleet.User{Teams: []fleet.UserTeam{{Team: fleet.Team{ID: 2}, Role: fleet.RoleObserver}}})
+		err := auth.AuthorizeOrNotFound(ctx, teamHost, fleet.ActionWrite, teamHost, notFoundErr)
+		require.Error(t, err)
+		require.ErrorIs(t, err, notFoundErr)
+	})
+
+	t.Run("nil notFoundErr never fails open", func(t *testing.T) {
+		// A caller misusing this helper by passing a nil notFoundErr must
+		// never get nil (success) back for a caller who can neither read nor
+		// write the resource: that would silently bypass authorization.
+		ctx := test.UserContext(t.Context(), &fleet.User{Teams: []fleet.UserTeam{{Team: fleet.Team{ID: 2}, Role: fleet.RoleObserver}}})
+		err := auth.AuthorizeOrNotFound(ctx, teamHost, fleet.ActionWrite, teamHost, nil)
+		require.Error(t, err)
+		var forbidden *Forbidden
+		require.ErrorAs(t, err, &forbidden)
+	})
+}

server/service/hosts.go

@@ -34,6 +34,7 @@ import (
 	"github.com/fleetdm/fleet/v4/server/mdm/assets"
 	mdmlifecycle "github.com/fleetdm/fleet/v4/server/mdm/lifecycle"
 	"github.com/fleetdm/fleet/v4/server/mdm/nanodep/godep"
+	common_mysql "github.com/fleetdm/fleet/v4/server/platform/mysql"
 	"github.com/fleetdm/fleet/v4/server/ptr"
 	"github.com/fleetdm/fleet/v4/server/worker"
 	"github.com/gocarina/gocsv"
@@ -915,8 +916,8 @@ func (svc *Service) checkWriteForHostIDs(ctx context.Context, ids []uint) error
 			return ctxerr.Wrap(ctx, err, "get host for delete")
 		}
 
-		// Authorize again with team loaded now that we have team_id
-		if err := svc.authz.Authorize(ctx, host, fleet.ActionWrite); err != nil {
+		notFoundErr := ctxerr.Wrap(ctx, common_mysql.NotFound("Host").WithID(id), "get host for delete")
+		if err := svc.authz.AuthorizeOrNotFound(ctx, host, fleet.ActionWrite, host, notFoundErr); err != nil {
 			return err
 		}
 	}
@@ -1113,8 +1114,13 @@ func (svc *Service) DeleteHost(ctx context.Context, id uint) error {
 		return ctxerr.Wrap(ctx, err, "get host for delete")
 	}
 
-	// Authorize again with team loaded now that we have team_id
-	if err := svc.authz.Authorize(ctx, host, fleet.ActionWrite); err != nil {
+	// Authorize again now that the host (and its team_id) is loaded. If the
+	// caller can't even read this host, it's entirely outside their
+	// visibility: report the same not-found error as a missing host above,
+	// rather than a forbidden that would confirm the host exists on some
+	// other team.
+	notFoundErr := ctxerr.Wrap(ctx, common_mysql.NotFound("Host").WithID(id), "get host for delete")
+	if err := svc.authz.AuthorizeOrNotFound(ctx, host, fleet.ActionWrite, host, notFoundErr); err != nil {
 		return err
 	}
 

server/service/hosts_test.go

@@ -1030,6 +1030,22 @@ func TestHostDetailsHostNameStatus(t *testing.T) {
 	})
 }
 
+// checkHostWriteAuthErr asserts the result of a host-mutation authorization
+// check. A caller with no read visibility into the host at all must see a
+// NotFound (masking existence), not a Forbidden that would confirm the host
+// exists on some other team; a caller who CAN read the host (e.g. same-team,
+// wrong role) still gets the normal Forbidden, since no new information is
+// disclosed by it.
+func checkHostWriteAuthErr(t *testing.T, shouldFail, expectNotFound bool, err error) {
+	t.Helper()
+	if shouldFail && expectNotFound {
+		require.Error(t, err)
+		assert.True(t, fleet.IsNotFound(err))
+		return
+	}
+	checkAuthErr(t, shouldFail, err)
+}
+
 // Fragile test: This test is fragile because of the large reliance on Datastore mocks. Consider refactoring test/logic or removing the test. It may be slowing us down more than helping us.
 func TestHostAuth(t *testing.T) {
 	ds := new(mock.Store)
@@ -1252,6 +1268,19 @@ func TestHostAuth(t *testing.T) {
 				IncludePolicies:  false,
 			}
 
+			// A team-only role never has read visibility into a host outside
+			// its own team(s) (including the team_id-less "global" host used
+			// below): a write-authz failure in that case must surface as
+			// NotFound rather than Forbidden, so it doesn't confirm the
+			// host's existence to a caller with no view into it.
+			isTeamOnlyRole := tt.user.GlobalRole == nil
+			belongsToTeam1 := false
+			for _, ut := range tt.user.Teams {
+				if ut.Team.ID == 1 {
+					belongsToTeam1 = true
+				}
+			}
+
 			_, err := svc.GetHost(ctx, 1, opts)
 			checkAuthErr(t, tt.shouldFailTeamRead, err)
 
@@ -1277,16 +1306,16 @@ func TestHostAuth(t *testing.T) {
 			checkAuthErr(t, tt.shouldFailGlobalRead, err)
 
 			err = svc.DeleteHost(ctx, 1)
-			checkAuthErr(t, tt.shouldFailTeamWrite, err)
+			checkHostWriteAuthErr(t, tt.shouldFailTeamWrite, isTeamOnlyRole && !belongsToTeam1, err)
 
 			err = svc.DeleteHost(ctx, 2)
-			checkAuthErr(t, tt.shouldFailGlobalWrite, err)
+			checkHostWriteAuthErr(t, tt.shouldFailGlobalWrite, isTeamOnlyRole, err)
 
 			err = svc.DeleteHosts(ctx, []uint{1}, nil)
-			checkAuthErr(t, tt.shouldFailTeamWrite, err)
+			checkHostWriteAuthErr(t, tt.shouldFailTeamWrite, isTeamOnlyRole && !belongsToTeam1, err)
 
 			err = svc.DeleteHosts(ctx, []uint{2}, nil)
-			checkAuthErr(t, tt.shouldFailGlobalWrite, err)
+			checkHostWriteAuthErr(t, tt.shouldFailGlobalWrite, isTeamOnlyRole, err)
 
 			err = svc.AddHostsToTeam(ctx, new(uint(1)), []uint{1}, false)
 			checkAuthErr(t, tt.shouldFailTeamWrite, err)
@@ -1692,6 +1721,42 @@ func TestDeleteHost(t *testing.T) {
 	})
 }
 
+func TestDeleteHostDoesNotLeakOutOfScopeExistence(t *testing.T) {
+	ds := new(mock.Store)
+	svc, ctx := newTestService(t, ds, nil, nil)
+
+	teamHost := &fleet.Host{ID: 1, TeamID: new(uint(1))}
+
+	ds.AppConfigFunc = func(ctx context.Context) (*fleet.AppConfig, error) {
+		return &fleet.AppConfig{}, nil
+	}
+	ds.HostLiteFunc = func(ctx context.Context, id uint) (*fleet.Host, error) {
+		return teamHost, nil
+	}
+
+	// A team-scoped observer with no relationship to team 1 can neither read
+	// nor write host 1: the response must be indistinguishable from a
+	// nonexistent host (NotFound), not a Forbidden that would confirm the
+	// host exists on some other team.
+	outOfScopeUser := &fleet.User{Teams: []fleet.UserTeam{{Team: fleet.Team{ID: 2}, Role: fleet.RoleObserver}}}
+	err := svc.DeleteHost(viewer.NewContext(ctx, viewer.Viewer{User: outOfScopeUser}), 1)
+	require.Error(t, err)
+	assert.True(t, fleet.IsNotFound(err))
+
+	err = svc.DeleteHosts(viewer.NewContext(ctx, viewer.Viewer{User: outOfScopeUser}), []uint{1}, nil)
+	require.Error(t, err)
+	assert.True(t, fleet.IsNotFound(err))
+
+	// A team-scoped observer who belongs to team 1 can read host 1, just not
+	// write it: this must remain a normal Forbidden error, since no new
+	// information about the host's existence is disclosed by it.
+	inScopeObserver := &fleet.User{Teams: []fleet.UserTeam{{Team: fleet.Team{ID: 1}, Role: fleet.RoleObserver}}}
+	err = svc.DeleteHost(viewer.NewContext(ctx, viewer.Viewer{User: inScopeObserver}), 1)
+	require.Error(t, err)
+	assert.False(t, fleet.IsNotFound(err))
+	assert.Contains(t, err.Error(), authz.ForbiddenErrorMessage)
+}
+
 func TestDeleteHostCreatesActivity(t *testing.T) {
 	ds := mysqltest.CreateMySQLDS(t)
 	defer ds.Close()