## Summary Fixes a security issue where `POST /api/latest/fleet/targets` returned sensitive fleet configuration to users with insufficient privileges. Other team-facing endpoints apply proper access controls; the target search path did not. - Replaces the `teamSearchResult` struct with a slim version containing only the non-sensitive fields documented in the API response (`id`, `created_at`, `name`, `description`, `user_count`, `host_count`, `display_text`, `count`). - Removes the `MarshalJSON`/`UnmarshalJSON` methods (~70 lines) that serialized fields the target picker never uses. - Verified that no frontend component, fleetctl client, or integration test reads sensitive fields from the target search response. - Validated the response shape matches the documented API contract in `docs/REST API/rest-api.md`. Closes fleetdm/confidential#16054 Related advisory: GHSA-88p2-jj8w-j8qg ## How we reproduced 1. Started local dev server (`fleet serve --dev --dev_license`) 2. Created a global observer user and a saved query with `observer_can_run = true` 3. Logged in as the observer **Before fix** -- same observer session, same team: ``` GET /api/latest/fleet/fleets/2/secrets -> secret: "********" (correctly masked) POST /api/latest/fleet/targets {"query":"","query_id":7,"selected":{"hosts":[],"labels":[],"teams":[]}} -> sensitive configuration leaked for all teams ``` **After fix** -- rebuilt binary, restarted server, same observer: ``` GET /api/latest/fleet/fleets/2/secrets -> secret: "********" (unchanged) POST /api/latest/fleet/targets (same request) -> only non-sensitive fields returned (id, name, display_text, count, etc.) ``` Also verified admin target search still returns team metadata correctly. ## Test plan - [x] Manual reproduction on local dev server - [x] Manual verification after fix - [x] Admin target search still returns team metadata (id, name, host_count, display_text) - [x] Verified no consumers (frontend, fleetctl, tests) read sensitive fields from target search - [x] Validated response matches documented API contract in `docs/REST API/rest-api.md` - [x] Unit test verifies response contains only documented non-sensitive fields - [x] `go test ./server/service/ -run TestSearchTargets` passes - [ ] CI passes
137 lines
4.3 KiB
Go
137 lines
4.3 KiB
Go
package service
|
|
|
|
import (
|
|
"context"
|
|
"encoding/json"
|
|
"testing"
|
|
|
|
"github.com/fleetdm/fleet/v4/server/contexts/viewer"
|
|
"github.com/fleetdm/fleet/v4/server/fleet"
|
|
"github.com/fleetdm/fleet/v4/server/mock"
|
|
"github.com/fleetdm/fleet/v4/server/ptr"
|
|
"github.com/stretchr/testify/assert"
|
|
"github.com/stretchr/testify/require"
|
|
)
|
|
|
|
func TestSearchTargets(t *testing.T) {
|
|
ds := new(mock.Store)
|
|
svc, ctx := newTestService(t, ds, nil, nil)
|
|
|
|
user := &fleet.User{GlobalRole: ptr.String(fleet.RoleAdmin)}
|
|
ctx = viewer.NewContext(ctx, viewer.Viewer{User: user})
|
|
|
|
hosts := []*fleet.Host{
|
|
{Hostname: "foo.local"},
|
|
}
|
|
labels := []*fleet.Label{
|
|
{
|
|
Name: "label foo",
|
|
Query: "query foo",
|
|
},
|
|
}
|
|
teams := []*fleet.Team{
|
|
{Name: "team1"},
|
|
}
|
|
|
|
ds.SearchHostsFunc = func(ctx context.Context, filter fleet.TeamFilter, query string, omit ...uint) ([]*fleet.Host, error) {
|
|
assert.Equal(t, user, filter.User)
|
|
return hosts, nil
|
|
}
|
|
ds.SearchLabelsFunc = func(ctx context.Context, filter fleet.TeamFilter, query string, omit ...uint) ([]*fleet.Label, error) {
|
|
assert.Equal(t, user, filter.User)
|
|
return labels, nil
|
|
}
|
|
ds.SearchTeamsFunc = func(ctx context.Context, filter fleet.TeamFilter, query string, omit ...uint) ([]*fleet.Team, error) {
|
|
assert.Equal(t, user, filter.User)
|
|
return teams, nil
|
|
}
|
|
|
|
results, err := svc.SearchTargets(ctx, "foo", nil, fleet.HostTargets{})
|
|
require.NoError(t, err)
|
|
assert.Equal(t, hosts[0], results.Hosts[0])
|
|
assert.Equal(t, labels[0], results.Labels[0])
|
|
assert.Equal(t, teams[0], results.Teams[0])
|
|
}
|
|
|
|
func TestSearchTargetsStripsSecretsAndAgentOptions(t *testing.T) {
|
|
ds := new(mock.Store)
|
|
svc, ctx := newTestService(t, ds, nil, nil)
|
|
|
|
// Use an observer role to mirror the vulnerable scenario.
|
|
user := &fleet.User{GlobalRole: new(fleet.RoleObserver)}
|
|
ctx = viewer.NewContext(ctx, viewer.Viewer{User: user})
|
|
|
|
agentOpts := json.RawMessage(`{"config":{"options":{"aws_secret_access_key":"SECRET"}}}`)
|
|
teams := []*fleet.Team{
|
|
{
|
|
ID: 1,
|
|
Name: "team1",
|
|
Config: fleet.TeamConfig{
|
|
AgentOptions: &agentOpts,
|
|
},
|
|
Secrets: []*fleet.EnrollSecret{
|
|
{Secret: "super-secret-token", TeamID: new(uint(1))},
|
|
},
|
|
},
|
|
{
|
|
ID: 2,
|
|
Name: "team2",
|
|
Secrets: []*fleet.EnrollSecret{
|
|
{Secret: "another-secret", TeamID: new(uint(2))},
|
|
},
|
|
},
|
|
}
|
|
|
|
ds.SearchHostsFunc = func(ctx context.Context, filter fleet.TeamFilter, query string, omit ...uint) ([]*fleet.Host, error) {
|
|
return nil, nil
|
|
}
|
|
ds.SearchLabelsFunc = func(ctx context.Context, filter fleet.TeamFilter, query string, omit ...uint) ([]*fleet.Label, error) {
|
|
return nil, nil
|
|
}
|
|
ds.SearchTeamsFunc = func(ctx context.Context, filter fleet.TeamFilter, query string, omit ...uint) ([]*fleet.Team, error) {
|
|
return teams, nil
|
|
}
|
|
|
|
results, err := svc.SearchTargets(ctx, "", nil, fleet.HostTargets{})
|
|
require.NoError(t, err)
|
|
require.Len(t, results.Teams, 2)
|
|
|
|
for _, team := range results.Teams {
|
|
assert.Nil(t, team.Secrets, "secrets should be stripped from team %s", team.Name)
|
|
assert.Nil(t, team.Config.AgentOptions, "agent_options should be stripped from team %s", team.Name)
|
|
}
|
|
|
|
// Verify non-sensitive fields are preserved.
|
|
assert.Equal(t, uint(1), results.Teams[0].ID)
|
|
assert.Equal(t, "team1", results.Teams[0].Name)
|
|
assert.Equal(t, uint(2), results.Teams[1].ID)
|
|
assert.Equal(t, "team2", results.Teams[1].Name)
|
|
}
|
|
|
|
func TestSearchWithOmit(t *testing.T) {
|
|
ds := new(mock.Store)
|
|
svc, ctx := newTestService(t, ds, nil, nil)
|
|
|
|
user := &fleet.User{GlobalRole: ptr.String(fleet.RoleAdmin)}
|
|
ctx = viewer.NewContext(ctx, viewer.Viewer{User: user})
|
|
|
|
ds.SearchHostsFunc = func(ctx context.Context, filter fleet.TeamFilter, query string, omit ...uint) ([]*fleet.Host, error) {
|
|
assert.Equal(t, user, filter.User)
|
|
assert.Equal(t, []uint{1, 2}, omit)
|
|
return nil, nil
|
|
}
|
|
ds.SearchLabelsFunc = func(ctx context.Context, filter fleet.TeamFilter, query string, omit ...uint) ([]*fleet.Label, error) {
|
|
assert.Equal(t, user, filter.User)
|
|
assert.Equal(t, []uint{3, 4}, omit)
|
|
return nil, nil
|
|
}
|
|
ds.SearchTeamsFunc = func(ctx context.Context, filter fleet.TeamFilter, query string, omit ...uint) ([]*fleet.Team, error) {
|
|
assert.Equal(t, user, filter.User)
|
|
assert.Equal(t, []uint{5, 6}, omit)
|
|
return nil, nil
|
|
}
|
|
|
|
_, err := svc.SearchTargets(ctx, "foo", nil, fleet.HostTargets{HostIDs: []uint{1, 2}, LabelIDs: []uint{3, 4}, TeamIDs: []uint{5, 6}})
|
|
require.NoError(t, err)
|
|
}
|