From 1c522097d03e2a4c1c427cd5dbbc2d58601c9df5 Mon Sep 17 00:00:00 2001 From: Sharon Katz <121527325+sharon-fdm@users.noreply.github.com> Date: Tue, 5 May 2026 16:19:41 -0400 Subject: [PATCH] Fix missing GitOps label validation for invalid field combinations (#44410) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit **Related issue:** Closes #34229 - [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 --- `fleetctl gitops` silently accepted labels with invalid parameter combinations (e.g. manual labels with query/criteria/platform). Added per-type field validation in a centralized `fleet.ValidateLabelMembershipFields` function, called from the GitOps parser, `ApplyLabelSpecs`, and `NewLabel`. | Type | Allowed | Now rejects | |------|---------|-------------| | `manual` | `name`, `description`, `hosts` | `query`, `criteria`, `platform` | | `dynamic` | `name`, `description`, `query`, `platform` | `criteria`, `hosts`; validates platform value | | `host_vitals` | `name`, `description`, `criteria` | `query`, `platform`, `hosts` | ### Automated tests - `TestLabelInvalidFieldCombinations` in `pkg/spec/gitops_test.go` — 17 sub-tests covering every invalid combination per label type, plus 3 valid happy-path cases. - `TestNewLabelFieldValidation` in `server/service/labels_test.go` — 4 cases for NewLabel validation. - `TestApplyLabelSpecsManualLabelNilHosts` — 10 sub-cases for ApplyLabelSpecs field validation. - `TestWhenCreatingNewLabelsPlatformIsValidated` — platform validation across NewLabel and ApplyLabelSpecs. All existing `pkg/spec` and `server/service` label tests pass. ## Summary by CodeRabbit * **Bug Fixes** * Labels now reject invalid field combinations for manual, dynamic, and host_vitals types with clear error responses instead of failing silently. * **Tests** * Added comprehensive tests covering valid and invalid label configurations across membership types. * **Documentation** * Changelog entry describing the behavioral fix. * **Chores** * Removed an unnecessary platform constraint from a label configuration. --- ### Manual test results Ran against a local Fleet server with the built binary. **API - NewLabel (POST /api/latest/fleet/labels)** | Test | Input | Expected | Result | |------|-------|----------|--------| | 1 | manual + platform=darwin | 422, field=`platform` | PASS | | 2 | dynamic + platform=invalidplatform | 422, field=`platform` | PASS | | 3 | dynamic + platform=darwin + query | 200 | PASS | | 4 | manual (no platform) | 200 | PASS | | 5 | host_vitals + platform=darwin | 422, field=`platform` | PASS | | 6 | dynamic + whitespace-only query | 422, field=`query` | PASS | **API - ApplyLabelSpecs (POST /api/latest/fleet/spec/labels)** | Test | Input | Expected | Result | |------|-------|----------|--------| | 7 | manual + query | 422, field=`query` | PASS | | 8 | dynamic + hosts | 422, field=`hosts` | PASS | | 9 | valid dynamic | 200 | PASS | **Round-trip: get labels --yaml then apply** | Test | Scenario | Result | |------|----------|--------| | 10 | Legacy manual label with platform=darwin in DB | Platform stripped from YAML, re-apply succeeds — PASS | | 11 | Dynamic label with platform=darwin | Platform preserved in YAML, re-apply succeeds — PASS | **GitOps parser (fleetctl gitops --dry-run)** | Test | Input | Result | |------|-------|--------| | 12 | manual + query + platform + criteria | All 3 errors surfaced at once — PASS | | 13 | valid manual label | No validation errors — PASS | | 14 | dynamic + invalid platform | Error surfaced — PASS | --- ### Code walkthrough **`server/fleet/labels.go`** — Added `ValidateLabelMembershipFields(*LabelSpec) *InvalidArgumentError`. This is the single source of truth for label field validation, returning field-specific errors (`platform`, `query`, `criteria`, `hosts`). Lives here because this package defines the label types both callers import. Also uses `strings.TrimSpace` to reject whitespace-only queries. **`server/service/labels.go`** — Three changes: (1) Removed the early blanket platform check from `NewLabel` that ran before the membership type was known. (2) Added `ValidateLabelMembershipFields` call in `NewLabel` after type inference, so the API rejects invalid combos at creation time. (3) Replaced three incomplete inline checks in `ApplyLabelSpecs` with a single call to the centralized function, using `err.WithStatus(422)` to preserve field-specific error shape in the API response. **`pkg/spec/gitops.go`** — Replaced the inline validation switch and a standalone `ValidLabelPlatformVariants` check with a call to `ValidateLabelMembershipFields`. Unwraps the returned errors individually into `multiError` so all validation problems are reported to the user at once. **`cmd/fleetctl/fleetctl/generate_gitops.go`** — Gated platform emission on `LabelMembershipTypeDynamic` so legacy manual/host_vitals labels with a stored platform don't produce YAML that fails re-import. **`cmd/fleetctl/fleetctl/get.go`** — Added `stripMismatchedLabelFields` which clears type-inappropriate fields (query, platform, criteria, hosts) per membership type before YAML output. Called in both code paths: listing all labels and fetching a single label by name. Ensures the `get labels --yaml` → `apply` round-trip works for legacy data. **`server/datastore/mysql/labels.go`** — Added missing `l.criteria` column to `GetLabelSpec` SELECT, matching `GetLabelSpecs`. Without it, host_vitals labels fetched by name lost their criteria in the YAML output, causing re-import to fail with the new validation. --- changes/34229-gitops-label-validation | 1 + cmd/fleetctl/fleetctl/generate_gitops.go | 2 +- cmd/fleetctl/fleetctl/get.go | 20 +++ pkg/spec/gitops.go | 11 +- pkg/spec/gitops_test.go | 209 +++++++++++++++++++++++ server/datastore/mysql/labels.go | 2 +- server/fleet/labels.go | 51 ++++++ server/service/labels.go | 34 ++-- server/service/labels_test.go | 155 ++++++++++++++++- 9 files changed, 454 insertions(+), 31 deletions(-) create mode 100644 changes/34229-gitops-label-validation diff --git a/changes/34229-gitops-label-validation b/changes/34229-gitops-label-validation new file mode 100644 index 0000000000..5ca88ac922 --- /dev/null +++ b/changes/34229-gitops-label-validation @@ -0,0 +1 @@ +* Fixed `fleetctl gitops` silently accepting labels with invalid parameter combinations (e.g. manual labels with query/criteria/platform). diff --git a/cmd/fleetctl/fleetctl/generate_gitops.go b/cmd/fleetctl/fleetctl/generate_gitops.go index 384bfdde36..80cf4641b2 100644 --- a/cmd/fleetctl/fleetctl/generate_gitops.go +++ b/cmd/fleetctl/fleetctl/generate_gitops.go @@ -2251,7 +2251,7 @@ func (cmd *GenerateGitopsCommand) generateLabels(team *fleet.Team) ([]map[string jsonFieldName(t, "Description"): label.Description, jsonFieldName(t, "LabelMembershipType"): label.LabelMembershipType, } - if label.Platform != "" { + if label.LabelMembershipType == fleet.LabelMembershipTypeDynamic && label.Platform != "" { labelSpec[jsonFieldName(t, "Platform")] = label.Platform } switch label.LabelMembershipType { diff --git a/cmd/fleetctl/fleetctl/get.go b/cmd/fleetctl/fleetctl/get.go index 8b08f98dd5..7a59ad50ff 100644 --- a/cmd/fleetctl/fleetctl/get.go +++ b/cmd/fleetctl/fleetctl/get.go @@ -92,6 +92,24 @@ func printYaml(spec interface{}, writer io.Writer) error { return nil } +// stripMismatchedLabelFields clears fields that are not meaningful for the +// label's membership type so that exported YAML can be re-applied cleanly. +func stripMismatchedLabelFields(label *fleet.LabelSpec) { + switch label.LabelMembershipType { + case fleet.LabelMembershipTypeManual: + label.Query = "" + label.Platform = "" + label.HostVitalsCriteria = nil + case fleet.LabelMembershipTypeDynamic: + label.Hosts = nil + label.HostVitalsCriteria = nil + case fleet.LabelMembershipTypeHostVitals: + label.Query = "" + label.Platform = "" + label.Hosts = nil + } +} + func printLabel(c *cli.Context, label *fleet.LabelSpec) error { spec := specGeneric{ Kind: fleet.LabelKind, @@ -725,6 +743,7 @@ func getLabelsCommand() *cli.Command { if c.Bool(yamlFlagName) || c.Bool(jsonFlagName) { for _, label := range labels { + stripMismatchedLabelFields(label) printLabel(c, label) //nolint:errcheck } return nil @@ -761,6 +780,7 @@ func getLabelsCommand() *cli.Command { return err } + stripMismatchedLabelFields(label) printLabel(c, label) //nolint:errcheck return nil }, diff --git a/pkg/spec/gitops.go b/pkg/spec/gitops.go index e926b12b0f..e9e180ed1e 100644 --- a/pkg/spec/gitops.go +++ b/pkg/spec/gitops.go @@ -1405,18 +1405,17 @@ func parseLabels(top map[string]json.RawMessage, result *GitOps, baseDir string, multiError = multierror.Append(multiError, errors.New("name is required for each label")) } - if l.LabelMembershipType != fleet.LabelMembershipTypeManual && l.Query == "" && l.HostVitalsCriteria == nil { - multiError = multierror.Append(multiError, errors.New("a SQL query or host vitals criteria is required for each non-manual label")) + // Validate mutually exclusive field combinations per label membership type + if err := fleet.ValidateLabelMembershipFields(l); err != nil { + for _, inv := range err.Invalid() { + multiError = multierror.Append(multiError, fmt.Errorf("%s", inv["reason"])) + } } // Don't use non-ASCII if !isASCII(l.Name) { multiError = multierror.Append(multiError, fmt.Errorf("label name must be in ASCII: %s", l.Name)) } - - if _, ok := fleet.ValidLabelPlatformVariants[l.Platform]; !ok { - multiError = multierror.Append(multiError, fmt.Errorf("invalid platform for label %q: %s", l.Name, l.Platform)) - } // Check that host vitals criteria is valid if l.HostVitalsCriteria != nil { criteriaJson, err := json.Marshal(l.HostVitalsCriteria) diff --git a/pkg/spec/gitops_test.go b/pkg/spec/gitops_test.go index df5c9589ef..3755909e17 100644 --- a/pkg/spec/gitops_test.go +++ b/pkg/spec/gitops_test.go @@ -458,6 +458,215 @@ labels: assert.Empty(t, gitops.Labels[0].Hosts) } +func TestLabelInvalidFieldCombinations(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + label string + wantErrs []string + notWantErrs []string + }{ + { + name: "manual label with query", + label: ` +labels: + - name: bad + label_membership_type: manual + query: SELECT 1`, + wantErrs: []string{`label "bad" is declared as manual but contains a query`}, + }, + { + name: "manual label with criteria", + label: ` +labels: + - name: bad + label_membership_type: manual + criteria: + vital: end_user_idp_group + operator: "=" + value: Engineering`, + wantErrs: []string{`label "bad" is declared as manual but contains criteria`}, + }, + { + name: "manual label with platform", + label: ` +labels: + - name: bad + label_membership_type: manual + platform: darwin`, + wantErrs: []string{`label "bad" is declared as manual but contains a platform`}, + }, + { + name: "manual label with all invalid fields", + label: ` +labels: + - name: bad + label_membership_type: manual + query: SELECT 1 + platform: darwin + criteria: + vital: end_user_idp_group + operator: "=" + value: Engineering`, + wantErrs: []string{ + `label "bad" is declared as manual but contains a query`, + `label "bad" is declared as manual but contains criteria`, + `label "bad" is declared as manual but contains a platform`, + }, + }, + { + name: "dynamic label with criteria", + label: ` +labels: + - name: bad + label_membership_type: dynamic + query: SELECT 1 + criteria: + vital: end_user_idp_group + operator: "=" + value: Engineering`, + wantErrs: []string{`label "bad" is declared as dynamic but contains criteria`}, + }, + { + name: "dynamic label with hosts", + label: ` +labels: + - name: bad + label_membership_type: dynamic + query: SELECT 1 + hosts: + - host1`, + wantErrs: []string{`label "bad" is declared as dynamic but contains hosts`}, + }, + { + name: "dynamic label missing query", + label: ` +labels: + - name: bad + label_membership_type: dynamic`, + wantErrs: []string{`label "bad" is declared as dynamic but is missing a query`}, + }, + { + name: "dynamic label with invalid platform", + label: ` +labels: + - name: bad + label_membership_type: dynamic + query: SELECT 1 + platform: invalidplatform`, + wantErrs: []string{`label "bad" has invalid platform: "invalidplatform"`}, + }, + { + name: "dynamic label with valid platform is ok", + label: ` +labels: + - name: ok + label_membership_type: dynamic + query: SELECT 1 + platform: darwin`, + }, + { + name: "host_vitals label with query", + label: ` +labels: + - name: bad + label_membership_type: host_vitals + query: SELECT 1 + criteria: + vital: end_user_idp_group + operator: "=" + value: Engineering`, + wantErrs: []string{`label "bad" is declared as host_vitals but contains a query`}, + }, + { + name: "host_vitals label with platform", + label: ` +labels: + - name: bad + label_membership_type: host_vitals + platform: darwin + criteria: + vital: end_user_idp_group + operator: "=" + value: Engineering`, + wantErrs: []string{`label "bad" is declared as host_vitals but contains a platform`}, + }, + { + name: "host_vitals label with hosts", + label: ` +labels: + - name: bad + label_membership_type: host_vitals + criteria: + vital: end_user_idp_group + operator: "=" + value: Engineering + hosts: + - host1`, + wantErrs: []string{`label "bad" is declared as host_vitals but contains hosts`}, + }, + { + name: "host_vitals label missing criteria", + label: ` +labels: + - name: bad + label_membership_type: host_vitals`, + wantErrs: []string{`label "bad" is declared as host_vitals but is missing criteria`}, + }, + { + name: "valid manual label", + label: ` +labels: + - name: ok + label_membership_type: manual + hosts: + - host1`, + }, + { + name: "valid dynamic label", + label: ` +labels: + - name: ok + label_membership_type: dynamic + query: SELECT 1`, + }, + { + name: "valid host_vitals label", + label: ` +labels: + - name: ok + label_membership_type: host_vitals + criteria: + vital: end_user_idp_group + operator: "=" + value: Engineering`, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + config := getGlobalConfig([]string{}) + config += tt.label + _, err := gitOpsFromString(t, config) + if len(tt.wantErrs) == 0 { + require.NoError(t, err) + } else { + require.Error(t, err) + for _, wantErr := range tt.wantErrs { + require.ErrorContains(t, err, wantErr) + } + } + for _, notWantErr := range tt.notWantErrs { + if err != nil { + assert.NotContains(t, err.Error(), notWantErr) + } + } + }) + } +} + func TestDuplicateQueryNames(t *testing.T) { t.Parallel() config := getGlobalConfig([]string{"reports"}) diff --git a/server/datastore/mysql/labels.go b/server/datastore/mysql/labels.go index 60ffa6d5fb..21c2baa9fc 100644 --- a/server/datastore/mysql/labels.go +++ b/server/datastore/mysql/labels.go @@ -578,7 +578,7 @@ func (ds *Datastore) GetLabelSpecs(ctx context.Context, filter fleet.TeamFilter) func (ds *Datastore) GetLabelSpec(ctx context.Context, filter fleet.TeamFilter, name string) (*fleet.LabelSpec, error) { var specs []*fleet.LabelSpec query, params, err := applyLabelTeamFilter(` -SELECT l.id, l.name, l.description, l.query, l.platform, l.label_type, l.label_membership_type, l.team_id +SELECT l.id, l.name, l.description, l.query, l.platform, l.label_type, l.label_membership_type, l.criteria, l.team_id FROM labels l WHERE l.name = ?`, filter, name) if err != nil { diff --git a/server/fleet/labels.go b/server/fleet/labels.go index cde6819f32..fda08e6af6 100644 --- a/server/fleet/labels.go +++ b/server/fleet/labels.go @@ -147,6 +147,57 @@ var ValidLabelPlatformVariants = map[string]struct{}{ "centos": {}, } +// ValidateLabelMembershipFields checks that the fields on a label spec are +// consistent with its declared membership type. It returns an +// InvalidArgumentError with field-specific entries, or nil if valid. +func ValidateLabelMembershipFields(spec *LabelSpec) *InvalidArgumentError { + var invalid InvalidArgumentError + switch spec.LabelMembershipType { + case LabelMembershipTypeManual: + if spec.Query != "" { + invalid.Append("query", fmt.Sprintf("label %q is declared as manual but contains a query", spec.Name)) + } + if spec.HostVitalsCriteria != nil { + invalid.Append("criteria", fmt.Sprintf("label %q is declared as manual but contains criteria", spec.Name)) + } + if spec.Platform != "" { + invalid.Append("platform", fmt.Sprintf("label %q is declared as manual but contains a platform", spec.Name)) + } + case LabelMembershipTypeDynamic: + if strings.TrimSpace(spec.Query) == "" { + invalid.Append("query", fmt.Sprintf("label %q is declared as dynamic but is missing a query", spec.Name)) + } + if spec.HostVitalsCriteria != nil { + invalid.Append("criteria", fmt.Sprintf("label %q is declared as dynamic but contains criteria", spec.Name)) + } + if len(spec.Hosts) > 0 { + invalid.Append("hosts", fmt.Sprintf("label %q is declared as dynamic but contains hosts", spec.Name)) + } + if spec.Platform != "" { + if _, ok := ValidLabelPlatformVariants[spec.Platform]; !ok { + invalid.Append("platform", fmt.Sprintf("label %q has invalid platform: %q", spec.Name, spec.Platform)) + } + } + case LabelMembershipTypeHostVitals: + if spec.HostVitalsCriteria == nil { + invalid.Append("criteria", fmt.Sprintf("label %q is declared as host_vitals but is missing criteria", spec.Name)) + } + if spec.Query != "" { + invalid.Append("query", fmt.Sprintf("label %q is declared as host_vitals but contains a query", spec.Name)) + } + if spec.Platform != "" { + invalid.Append("platform", fmt.Sprintf("label %q is declared as host_vitals but contains a platform", spec.Name)) + } + if len(spec.Hosts) > 0 { + invalid.Append("hosts", fmt.Sprintf("label %q is declared as host_vitals but contains hosts", spec.Name)) + } + } + if invalid.HasErrors() { + return &invalid + } + return nil +} + type Label struct { UpdateCreateTimestamps ID uint `json:"id"` diff --git a/server/service/labels.go b/server/service/labels.go index 86bd7f5ab0..1da36dc869 100644 --- a/server/service/labels.go +++ b/server/service/labels.go @@ -48,10 +48,6 @@ func (svc *Service) NewLabel(ctx context.Context, p fleet.LabelPayload) (*fleet. return nil, nil, fleet.ErrNoContext } - if _, ok := fleet.ValidLabelPlatformVariants[p.Platform]; !ok { - return nil, nil, fleet.NewInvalidArgumentError("platform", fmt.Sprintf("invalid platform: %s", p.Platform)) - } - if len(p.Hosts) > 0 && len(p.HostIDs) > 0 { return nil, nil, fleet.NewInvalidArgumentError("hosts", `Only one of either "hosts" or "host_ids" can be included in the request.`) } @@ -97,6 +93,17 @@ func (svc *Service) NewLabel(ctx context.Context, p fleet.LabelPayload) (*fleet. label.Platform = p.Platform label.Description = p.Description + // Validate field combinations for the inferred membership type + if err := fleet.ValidateLabelMembershipFields(&fleet.LabelSpec{ + Name: label.Name, + Query: label.Query, + Platform: label.Platform, + LabelMembershipType: label.LabelMembershipType, + HostVitalsCriteria: label.HostVitalsCriteria, + }); err != nil { + return nil, nil, err + } + for name := range fleet.ReservedLabelNames() { if label.Name == name { return nil, nil, fleet.NewInvalidArgumentError("name", fmt.Sprintf("cannot add label '%s' because it conflicts with the name of a built-in label", name)) @@ -615,22 +622,9 @@ func (svc *Service) ApplyLabelSpecs(ctx context.Context, specs []*fleet.LabelSpe var specLabelNamesNeedingMoving []string // should match namesToMove once specs have been checked for _, spec := range specs { - if _, ok := fleet.ValidLabelPlatformVariants[spec.Platform]; !ok { - return fleet.NewUserMessageError( - ctxerr.Errorf(ctx, "invalid platform: %s", spec.Platform), http.StatusUnprocessableEntity, - ) - } - - if spec.LabelMembershipType == fleet.LabelMembershipTypeDynamic && len(spec.Hosts) > 0 { - return fleet.NewUserMessageError( - ctxerr.Errorf(ctx, "label %s is declared as dynamic but contains `hosts` key", spec.Name), http.StatusUnprocessableEntity, - ) - } - if spec.LabelMembershipType == fleet.LabelMembershipTypeHostVitals && spec.HostVitalsCriteria == nil { - // Criteria is required for host vitals labels. - return fleet.NewUserMessageError( - ctxerr.Errorf(ctx, "label %s is declared as host vitals but contains no `criteria` key", spec.Name), http.StatusUnprocessableEntity, - ) + // Validate mutually exclusive field combinations per label membership type + if err := fleet.ValidateLabelMembershipFields(spec); err != nil { + return err.WithStatus(http.StatusUnprocessableEntity) } if spec.LabelType == fleet.LabelTypeBuiltIn { // We allow specs to contain built-in labels as long as they are not being modified. diff --git a/server/service/labels_test.go b/server/service/labels_test.go index e226a299b1..2eb78855ae 100644 --- a/server/service/labels_test.go +++ b/server/service/labels_test.go @@ -94,7 +94,7 @@ func TestWhenCreatingNewLabelsPlatformIsValidated(t *testing.T) { Platform: platform, }) if tc.ShouldFail { - require.Contains(t, err.Error(), fmt.Sprintf("invalid platform: %s", platform)) + require.Contains(t, err.Error(), fmt.Sprintf("invalid platform: %q", platform)) } else { require.NoError(t, err) require.NotNil(t, actualNewLabel) @@ -109,7 +109,7 @@ func TestWhenCreatingNewLabelsPlatformIsValidated(t *testing.T) { Platform: platform, }}, nil, nil) if tc.ShouldFail { - require.Contains(t, err.Error(), fmt.Sprintf("invalid platform: %s", platform)) + require.Contains(t, err.Error(), fmt.Sprintf("invalid platform: %q", platform)) } else { require.NoError(t, err) } @@ -770,7 +770,108 @@ func TestApplyLabelSpecsManualLabelNilHosts(t *testing.T) { }, }, nil, nil) require.Error(t, err) - require.ErrorContains(t, err, "declared as dynamic but contains `hosts` key") + require.ErrorContains(t, err, "declared as dynamic but contains hosts") + + // Dynamic label without query should be rejected + err = svc.ApplyLabelSpecs(ctx, []*fleet.LabelSpec{ + { + Name: "dynamic_no_query", + LabelMembershipType: fleet.LabelMembershipTypeDynamic, + }, + }, nil, nil) + require.Error(t, err) + require.ErrorContains(t, err, "declared as dynamic but is missing a query") + + // Dynamic label with criteria should be rejected + err = svc.ApplyLabelSpecs(ctx, []*fleet.LabelSpec{ + { + Name: "dynamic_with_criteria", + Query: "SELECT 1", + LabelMembershipType: fleet.LabelMembershipTypeDynamic, + HostVitalsCriteria: new(json.RawMessage(`{"vital":"end_user_idp_group","operator":"=","value":"Engineering"}`)), + }, + }, nil, nil) + require.Error(t, err) + require.ErrorContains(t, err, "declared as dynamic but contains criteria") + + // Manual label with query should be rejected + err = svc.ApplyLabelSpecs(ctx, []*fleet.LabelSpec{ + { + Name: "manual_with_query", + LabelMembershipType: fleet.LabelMembershipTypeManual, + Query: "SELECT 1", + }, + }, nil, nil) + require.Error(t, err) + require.ErrorContains(t, err, "declared as manual but contains a query") + + // Manual label with criteria should be rejected + err = svc.ApplyLabelSpecs(ctx, []*fleet.LabelSpec{ + { + Name: "manual_with_criteria", + LabelMembershipType: fleet.LabelMembershipTypeManual, + HostVitalsCriteria: new(json.RawMessage(`{"vital":"end_user_idp_group","operator":"=","value":"Engineering"}`)), + }, + }, nil, nil) + require.Error(t, err) + require.ErrorContains(t, err, "declared as manual but contains criteria") + + // Manual label with platform should be rejected + err = svc.ApplyLabelSpecs(ctx, []*fleet.LabelSpec{ + { + Name: "manual_with_platform", + LabelMembershipType: fleet.LabelMembershipTypeManual, + Platform: "darwin", + }, + }, nil, nil) + require.Error(t, err) + require.ErrorContains(t, err, "declared as manual but contains a platform") + + // Host_vitals label without criteria should be rejected + err = svc.ApplyLabelSpecs(ctx, []*fleet.LabelSpec{ + { + Name: "host_vitals_no_criteria", + LabelMembershipType: fleet.LabelMembershipTypeHostVitals, + }, + }, nil, nil) + require.Error(t, err) + require.ErrorContains(t, err, "declared as host_vitals but is missing criteria") + + // Host_vitals label with query should be rejected + err = svc.ApplyLabelSpecs(ctx, []*fleet.LabelSpec{ + { + Name: "host_vitals_with_query", + LabelMembershipType: fleet.LabelMembershipTypeHostVitals, + HostVitalsCriteria: new(json.RawMessage(`{"vital":"end_user_idp_group","operator":"=","value":"Engineering"}`)), + Query: "SELECT 1", + }, + }, nil, nil) + require.Error(t, err) + require.ErrorContains(t, err, "declared as host_vitals but contains a query") + + // Host_vitals label with platform should be rejected + err = svc.ApplyLabelSpecs(ctx, []*fleet.LabelSpec{ + { + Name: "host_vitals_with_platform", + LabelMembershipType: fleet.LabelMembershipTypeHostVitals, + HostVitalsCriteria: new(json.RawMessage(`{"vital":"end_user_idp_group","operator":"=","value":"Engineering"}`)), + Platform: "darwin", + }, + }, nil, nil) + require.Error(t, err) + require.ErrorContains(t, err, "declared as host_vitals but contains a platform") + + // Host_vitals label with hosts should be rejected + err = svc.ApplyLabelSpecs(ctx, []*fleet.LabelSpec{ + { + Name: "host_vitals_with_hosts", + LabelMembershipType: fleet.LabelMembershipTypeHostVitals, + HostVitalsCriteria: new(json.RawMessage(`{"vital":"end_user_idp_group","operator":"=","value":"Engineering"}`)), + Hosts: []string{"host1"}, + }, + }, nil, nil) + require.Error(t, err) + require.ErrorContains(t, err, "declared as host_vitals but contains hosts") } func TestNewManualLabel(t *testing.T) { @@ -890,6 +991,54 @@ func TestNewHostVitalsLabel(t *testing.T) { }) } +func TestNewLabelFieldValidation(t *testing.T) { + ds := new(mock.Store) + svc, ctx := newTestService(t, ds, nil, nil) + ctx = viewer.NewContext(ctx, viewer.Viewer{User: &fleet.User{GlobalRole: ptr.String(fleet.RoleAdmin)}}) + + ds.NewLabelFunc = func(ctx context.Context, lbl *fleet.Label, opts ...fleet.OptionalArg) (*fleet.Label, error) { + lbl.ID = 1 + return lbl, nil + } + + // Manual label (no query) with platform should be rejected + _, _, err := svc.NewLabel(ctx, fleet.LabelPayload{ + Name: "manual_with_platform", + Platform: "darwin", + }) + require.Error(t, err) + require.ErrorContains(t, err, "declared as manual but contains a platform") + + // Host_vitals label with platform should be rejected + _, _, err = svc.NewLabel(ctx, fleet.LabelPayload{ + Name: "vitals_with_platform", + Platform: "darwin", + Criteria: &fleet.HostVitalCriteria{ + Vital: ptr.String("end_user_idp_group"), + Value: ptr.String("admin"), + }, + }) + require.Error(t, err) + require.ErrorContains(t, err, "declared as host_vitals but contains a platform") + + // Dynamic label with invalid platform should be rejected + _, _, err = svc.NewLabel(ctx, fleet.LabelPayload{ + Name: "dynamic_bad_platform", + Query: "SELECT 1", + Platform: "invalidplatform", + }) + require.Error(t, err) + require.ErrorContains(t, err, "invalid platform") + + // Dynamic label with valid platform should succeed + _, _, err = svc.NewLabel(ctx, fleet.LabelPayload{ + Name: "dynamic_good_platform", + Query: "SELECT 1", + Platform: "darwin", + }) + require.NoError(t, err) +} + func TestLabelActivities(t *testing.T) { ds := new(mock.Store) opts := &TestServerOpts{}