diff --git a/cmd/fleetctl/fleetctl/gitops.go b/cmd/fleetctl/fleetctl/gitops.go index 06261b691aa..8d73e4df8c1 100644 --- a/cmd/fleetctl/fleetctl/gitops.go +++ b/cmd/fleetctl/fleetctl/gitops.go @@ -974,7 +974,7 @@ func getLabelUsage(config *spec.GitOps) (map[string][]LabelUsage, error) { if len(setting.LabelsIncludeAll) > 0 { labels = setting.LabelsIncludeAll } - if overlap := fleet.ProfileLabelOverlap(labels, setting.LabelsExcludeAny); overlap != "" { + if overlap := fleet.LabelOverlap(labels, setting.LabelsExcludeAny); overlap != "" { return nil, fmt.Errorf("configuration profile '%s': label %q cannot appear in both include and exclude lists.", filepath.Base(setting.Path), overlap) } labels = append(labels, setting.LabelsExcludeAny...) diff --git a/server/datastore/mysql/hosts.go b/server/datastore/mysql/hosts.go index 52ff44057fd..2dac8c2673e 100644 --- a/server/datastore/mysql/hosts.go +++ b/server/datastore/mysql/hosts.go @@ -3758,7 +3758,11 @@ func (ds *Datastore) ListPoliciesForHost(ctx context.Context, host *fleet.Host) -- count of include_all labels this host is a member of SUM(CASE WHEN pl.exclude = 0 AND pl.require_all = 1 AND lm.host_id IS NOT NULL THEN 1 ELSE 0 END) AS host_include_all_count, -- 1 if this host is a member of at least one exclude_any label - MAX(CASE WHEN pl.exclude = 1 AND lm.host_id IS NOT NULL THEN 1 ELSE 0 END) AS host_in_exclude + MAX(CASE WHEN pl.exclude = 1 AND pl.require_all = 0 AND lm.host_id IS NOT NULL THEN 1 ELSE 0 END) AS host_in_exclude_any, + -- count of exclude_all labels on this policy + SUM(CASE WHEN pl.exclude = 1 AND pl.require_all = 1 THEN 1 ELSE 0 END) AS exclude_all_count, + -- count of exclude_all labels this host is a member of + SUM(CASE WHEN pl.exclude = 1 AND pl.require_all = 1 AND lm.host_id IS NOT NULL THEN 1 ELSE 0 END) AS host_exclude_all_count FROM policy_labels pl LEFT JOIN label_membership lm ON lm.label_id = pl.label_id AND lm.host_id = ? GROUP BY pl.policy_id @@ -3770,7 +3774,9 @@ func (ds *Datastore) ListPoliciesForHost(ctx context.Context, host *fleet.Host) -- Policy has no include_all labels, or host is in all of them AND (COALESCE(pl_agg.include_all_count, 0) = 0 OR pl_agg.host_include_all_count = pl_agg.include_all_count) -- Host is not in any exclude_any label - AND COALESCE(pl_agg.host_in_exclude, 0) = 0 + AND COALESCE(pl_agg.host_in_exclude_any, 0) = 0 + -- Policy has no exclude_all labels, or host is not in all of them + AND (COALESCE(pl_agg.exclude_all_count, 0) = 0 OR pl_agg.host_exclude_all_count < pl_agg.exclude_all_count) ORDER BY FIELD(response, 'fail', '', 'pass'), p.name` var policies []*fleet.HostPolicy diff --git a/server/datastore/mysql/policies.go b/server/datastore/mysql/policies.go index f770596c288..05cd4bc93ac 100644 --- a/server/datastore/mysql/policies.go +++ b/server/datastore/mysql/policies.go @@ -140,6 +140,7 @@ func newGlobalPolicy(ctx context.Context, db sqlx.ExtContext, authorID *uint, ar LabelsIncludeAny: fleet.LabelNamesToIdents(args.LabelsIncludeAny), LabelsIncludeAll: fleet.LabelNamesToIdents(args.LabelsIncludeAll), LabelsExcludeAny: fleet.LabelNamesToIdents(args.LabelsExcludeAny), + LabelsExcludeAll: fleet.LabelNamesToIdents(args.LabelsExcludeAll), }, } @@ -164,68 +165,73 @@ func updatePolicyLabelsTx(ctx context.Context, tx sqlx.ExtContext, policy *fleet WHERE name IN (?) ` - // Scopes are mutually exclusive - scopesSet := 0 - if len(policy.LabelsIncludeAny) > 0 { - scopesSet++ - } - if len(policy.LabelsIncludeAll) > 0 { - scopesSet++ - } - if len(policy.LabelsExcludeAny) > 0 { - scopesSet++ + if err := fleet.VerifyPolicyLabelScopes( + fleet.LabelIdentsToNames(policy.LabelsIncludeAny), + fleet.LabelIdentsToNames(policy.LabelsIncludeAll), + fleet.LabelIdentsToNames(policy.LabelsExcludeAny), + fleet.LabelIdentsToNames(policy.LabelsExcludeAll), + ); err != nil { + return ctxerr.Wrap(ctx, err, "validating policy label scopes") } - if scopesSet > 1 { - return ctxerr.Wrap(ctx, fleet.ErrPolicyConflictingLabels) + + if _, err := tx.ExecContext(ctx, deleteLabelsStmt, policy.ID); err != nil { + return ctxerr.Wrap(ctx, err, "deleting old policy labels") } - var ( - labelNames []string - exclude bool - requireAll bool - ) - switch { - case len(policy.LabelsIncludeAll) > 0: - requireAll = true - for _, label := range policy.LabelsIncludeAll { - labelNames = append(labelNames, label.LabelName) + // Each scope maps to a (exclude, require_all) pair in policy_labels: + // include_any -> exclude=0, require_all=0 + // include_all -> exclude=0, require_all=1 + // exclude_any -> exclude=1, require_all=0 + // exclude_all -> exclude=1, require_all=1 + insertScope := func(labels []fleet.LabelIdent, exclude, requireAll bool) error { + names := make([]string, 0, len(labels)) + for _, label := range labels { + names = append(names, label.LabelName) } - case len(policy.LabelsExcludeAny) > 0: - exclude = true - for _, label := range policy.LabelsExcludeAny { - labelNames = append(labelNames, label.LabelName) + + stmt, args, err := sqlx.In(insertLabelStmt, policy.ID, exclude, requireAll, names) + if err != nil { + return ctxerr.Wrap(ctx, err, "constructing policy label update query") } - default: - for _, label := range policy.LabelsIncludeAny { - labelNames = append(labelNames, label.LabelName) + + res, err := tx.ExecContext(ctx, stmt, args...) + if err != nil { + return ctxerr.Wrap(ctx, err, "creating policy labels") } - } - if _, err := tx.ExecContext(ctx, deleteLabelsStmt, policy.ID); err != nil { - return ctxerr.Wrap(ctx, err, "deleting old policy labels") - } + rowsAffected, err := res.RowsAffected() + if err != nil { + return ctxerr.Wrap(ctx, err, "listing number of policy labels affected") + } - if len(labelNames) == 0 { + if rowsAffected != int64(len(names)) { + return ctxerr.Errorf(ctx, "invalid label") + } return nil } - labelStmt, args, err := sqlx.In(insertLabelStmt, policy.ID, exclude, requireAll, labelNames) - if err != nil { - return ctxerr.Wrap(ctx, err, "constructing policy label update query") + // Only insert the scopes that actually have labels. PolicyPayload.Verify in the service layer + // enforces at most one include scope (any/all) and one exclude scope (any/all), so in practice + // at most two of these inserts run. + if len(policy.LabelsIncludeAny) > 0 { + if err := insertScope(policy.LabelsIncludeAny, false, false); err != nil { + return err + } } - - res, err := tx.ExecContext(ctx, labelStmt, args...) - if err != nil { - return ctxerr.Wrap(ctx, err, "creating policy labels") + if len(policy.LabelsIncludeAll) > 0 { + if err := insertScope(policy.LabelsIncludeAll, false, true); err != nil { + return err + } } - - rowsAffected, err := res.RowsAffected() - if err != nil { - return ctxerr.Wrap(ctx, err, "listing number of policy labels affected") + if len(policy.LabelsExcludeAny) > 0 { + if err := insertScope(policy.LabelsExcludeAny, true, false); err != nil { + return err + } } - - if rowsAffected != int64(len(labelNames)) { - return ctxerr.Errorf(ctx, "invalid label") + if len(policy.LabelsExcludeAll) > 0 { + if err := insertScope(policy.LabelsExcludeAll, true, true); err != nil { + return err + } } return nil @@ -255,6 +261,7 @@ func loadLabelsForPolicies(ctx context.Context, db sqlx.QueryerContext, policies policy.LabelsIncludeAny = nil policy.LabelsIncludeAll = nil policy.LabelsExcludeAny = nil + policy.LabelsExcludeAll = nil policyIDs = append(policyIDs, policy.ID) policyMap[policy.ID] = policy } @@ -279,7 +286,12 @@ func loadLabelsForPolicies(ctx context.Context, db sqlx.QueryerContext, policies for _, row := range rows { ident := fleet.LabelIdent{LabelName: row.LabelName, LabelID: row.LabelID} policy := policyMap[row.PolicyID] + if policy == nil { + continue + } switch { + case row.Exclude && row.RequireAll: + policy.LabelsExcludeAll = append(policy.LabelsExcludeAll, ident) case row.Exclude: policy.LabelsExcludeAny = append(policy.LabelsExcludeAny, ident) case row.RequireAll: @@ -1152,6 +1164,7 @@ func deletePolicyDB(ctx context.Context, q sqlx.ExtContext, ids []uint, teamID * // exclude=0, require_all=0 -> include_any // exclude=0, require_all=1 -> include_all // exclude=1, require_all=0 -> exclude_any +// exclude=1, require_all=1 -> exclude_all // // Placeholder order: lm.host_id, team_id, platform. policyQueriesForHostInScope appends "AND p.id IN (?)" (and its arg) to // restrict to specific policies. @@ -1169,7 +1182,11 @@ const policyQueriesForHostStmt = ` -- count of include_all labels this host is a member of SUM(CASE WHEN pl.exclude = 0 AND pl.require_all = 1 AND lm.host_id IS NOT NULL THEN 1 ELSE 0 END) AS host_include_all_count, -- 1 if this host is a member of at least one exclude_any label - MAX(CASE WHEN pl.exclude = 1 AND lm.host_id IS NOT NULL THEN 1 ELSE 0 END) AS host_in_exclude + MAX(CASE WHEN pl.exclude = 1 AND pl.require_all = 0 AND lm.host_id IS NOT NULL THEN 1 ELSE 0 END) AS host_in_exclude_any, + -- count of exclude_all labels on this policy + SUM(CASE WHEN pl.exclude = 1 AND pl.require_all = 1 THEN 1 ELSE 0 END) AS exclude_all_count, + -- count of exclude_all labels this host is a member of + SUM(CASE WHEN pl.exclude = 1 AND pl.require_all = 1 AND lm.host_id IS NOT NULL THEN 1 ELSE 0 END) AS host_exclude_all_count FROM policy_labels pl LEFT JOIN label_membership lm ON lm.label_id = pl.label_id AND lm.host_id = ? GROUP BY pl.policy_id @@ -1182,7 +1199,9 @@ const policyQueriesForHostStmt = ` -- Policy has no include_all labels, or host is in all of them (COALESCE(pl_agg.include_all_count, 0) = 0 OR pl_agg.host_include_all_count = pl_agg.include_all_count) AND -- Host is not in any exclude_any label - COALESCE(pl_agg.host_in_exclude, 0) = 0` + COALESCE(pl_agg.host_in_exclude_any, 0) = 0 AND + -- Policy has no exclude_all labels, or host is not in all of them + (COALESCE(pl_agg.exclude_all_count, 0) = 0 OR pl_agg.host_exclude_all_count < pl_agg.exclude_all_count)` // PolicyQueriesForHost returns the policy queries that are to be executed on the given host. func (ds *Datastore) PolicyQueriesForHost(ctx context.Context, host *fleet.Host) (map[string]string, error) { @@ -1384,6 +1403,7 @@ func newTeamPolicy(ctx context.Context, db sqlx.ExtContext, teamID uint, authorI LabelsIncludeAny: fleet.LabelNamesToIdents(args.LabelsIncludeAny), LabelsIncludeAll: fleet.LabelNamesToIdents(args.LabelsIncludeAll), LabelsExcludeAny: fleet.LabelNamesToIdents(args.LabelsExcludeAny), + LabelsExcludeAll: fleet.LabelNamesToIdents(args.LabelsExcludeAll), }, } @@ -1808,6 +1828,7 @@ func (ds *Datastore) ApplyPolicySpecs(ctx context.Context, authorID uint, specs LabelsIncludeAny: fleet.LabelNamesToIdents(spec.LabelsIncludeAny), LabelsIncludeAll: fleet.LabelNamesToIdents(spec.LabelsIncludeAll), LabelsExcludeAny: fleet.LabelNamesToIdents(spec.LabelsExcludeAny), + LabelsExcludeAll: fleet.LabelNamesToIdents(spec.LabelsExcludeAll), }, }) if err != nil { @@ -2154,7 +2175,22 @@ func cleanupPolicyMembershipOnPolicyUpdate( AND NOT EXISTS ( SELECT 1 FROM policy_labels pl JOIN label_membership lm ON lm.label_id = pl.label_id AND lm.host_id = pm.host_id - WHERE pl.policy_id = pm.policy_id AND pl.exclude = 1 + WHERE pl.policy_id = pm.policy_id AND pl.exclude = 1 AND pl.require_all = 0 + ) + -- If the policy has exclude_all labels, the host must not be in all of them. + AND ( + NOT EXISTS ( + SELECT 1 FROM policy_labels pl + WHERE pl.policy_id = pm.policy_id AND pl.exclude = 1 AND pl.require_all = 1 + ) + OR ( + SELECT COUNT(*) FROM policy_labels pl + WHERE pl.policy_id = pm.policy_id AND pl.exclude = 1 AND pl.require_all = 1 + ) > ( + SELECT COUNT(*) FROM policy_labels pl + JOIN label_membership lm ON lm.label_id = pl.label_id AND lm.host_id = pm.host_id + WHERE pl.policy_id = pm.policy_id AND pl.exclude = 1 AND pl.require_all = 1 + ) ) ) ORDER BY pm.host_id ASC diff --git a/server/datastore/mysql/policies_test.go b/server/datastore/mysql/policies_test.go index 11c6550c431..89603ff0445 100644 --- a/server/datastore/mysql/policies_test.go +++ b/server/datastore/mysql/policies_test.go @@ -760,17 +760,21 @@ func testTeamPolicyProprietary(t *testing.T, ds *Datastore) { require.NoError(t, err) requireLabels(t, []string{label1.Name, label2.Name}, gpol.LabelsIncludeAny) - // Cannot create a policy with inclusive and exclusive labels set + // A policy can combine an include scope with an exclude scope; both persist. gpol1, err := ds.NewGlobalPolicy(ctx, &user1.ID, fleet.PolicyPayload{ - Name: "global-query-bad-both-labels", + Name: "global-query-both-labels", Query: "select 1;", Description: "query1 desc", Resolution: "query1 resolution", - LabelsExcludeAny: []string{label1.Name}, LabelsIncludeAny: []string{label2.Name}, + LabelsExcludeAny: []string{label1.Name}, }) - require.Error(t, err) - require.Nil(t, gpol1) + require.NoError(t, err) + requireLabels(t, []string{label2.Name}, gpol1.LabelsIncludeAny) + requireLabels(t, []string{label1.Name}, gpol1.LabelsExcludeAny) + // Remove it so the global-policy count assertions below still hold. + _, err = ds.DeleteGlobalPolicies(ctx, []uint{gpol1.ID}) + require.NoError(t, err) // Cannot create policy with invalid label set gpol1, err = ds.NewGlobalPolicy(ctx, &user1.ID, fleet.PolicyPayload{ @@ -896,17 +900,36 @@ func testTeamPolicyProprietary(t *testing.T, ds *Datastore) { require.Error(t, err) require.Nil(t, p3) - // Can't create a policy with both include and excldue any labels - p3, err = ds.NewTeamPolicy(ctx, team1.ID, &user1.ID, fleet.PolicyPayload{ - Name: "query-bothlabel", + // A policy can combine an include scope with an exclude scope; both persist + // and round-trip from the DB. + pCombined, err := ds.NewTeamPolicy(ctx, team1.ID, &user1.ID, fleet.PolicyPayload{ + Name: "query-combined", Query: "select 2;", - Description: "query2 other description", - Resolution: "query2 other resolution", + Description: "combined description", + Resolution: "combined resolution", + LabelsIncludeAny: []string{label2.Name}, LabelsExcludeAny: []string{label1.Name}, + }) + require.NoError(t, err) + requireLabels(t, []string{label2.Name}, pCombined.LabelsIncludeAny) + requireLabels(t, []string{label1.Name}, pCombined.LabelsExcludeAny) + pCombinedReloaded, err := ds.Policy(ctx, pCombined.ID) + require.NoError(t, err) + requireLabels(t, []string{label2.Name}, pCombinedReloaded.LabelsIncludeAny) + requireLabels(t, []string{label1.Name}, pCombinedReloaded.LabelsExcludeAny) + _, err = ds.DeleteTeamPolicies(ctx, team1.ID, []uint{pCombined.ID}) + require.NoError(t, err) + + // The datastore guard (updatePolicyLabelsTx) rejects invalid scope + // combinations even when a caller skips PolicyPayload.Verify: two include + // scopes is not allowed. + _, err = ds.NewTeamPolicy(ctx, team1.ID, &user1.ID, fleet.PolicyPayload{ + Name: "query-two-include-scopes", + Query: "select 2;", LabelsIncludeAny: []string{label2.Name}, + LabelsIncludeAll: []string{label1.Name}, }) - require.Error(t, err) - require.Nil(t, p3) + require.ErrorIs(t, err, fleet.ErrPolicyConflictingIncludeLabels) // Can create a policy with include_all labels. pIncludeAll, err := ds.NewTeamPolicy(ctx, team1.ID, &user1.ID, fleet.PolicyPayload{ @@ -929,23 +952,23 @@ func testTeamPolicyProprietary(t *testing.T, ds *Datastore) { require.Empty(t, pReloaded.LabelsIncludeAny) require.Empty(t, pReloaded.LabelsExcludeAny) - // Three-way mutex: include_all + include_any rejected. - _, err = ds.NewTeamPolicy(ctx, team1.ID, &user1.ID, fleet.PolicyPayload{ - Name: "query-include-all-and-any", - Query: "select 1;", - LabelsIncludeAll: []string{label1.Name}, - LabelsIncludeAny: []string{label2.Name}, - }) - require.Error(t, err) - - // Three-way mutex: include_all + exclude_any rejected. - _, err = ds.NewTeamPolicy(ctx, team1.ID, &user1.ID, fleet.PolicyPayload{ - Name: "query-include-all-and-exclude", + // A policy can combine include_all with exclude_all; both persist and + // round-trip, exercising the exclude_all encoding (exclude=1, require_all=1). + pExclAll, err := ds.NewTeamPolicy(ctx, team1.ID, &user1.ID, fleet.PolicyPayload{ + Name: "query-include-all-exclude-all", Query: "select 1;", LabelsIncludeAll: []string{label1.Name}, - LabelsExcludeAny: []string{label2.Name}, + LabelsExcludeAll: []string{label2.Name}, }) - require.Error(t, err) + require.NoError(t, err) + requireLabels(t, []string{label1.Name}, pExclAll.LabelsIncludeAll) + requireLabels(t, []string{label2.Name}, pExclAll.LabelsExcludeAll) + pExclAllReloaded, err := ds.Policy(ctx, pExclAll.ID) + require.NoError(t, err) + requireLabels(t, []string{label1.Name}, pExclAllReloaded.LabelsIncludeAll) + requireLabels(t, []string{label2.Name}, pExclAllReloaded.LabelsExcludeAll) + _, err = ds.DeleteTeamPolicies(ctx, team1.ID, []uint{pExclAll.ID}) + require.NoError(t, err) // Cleanup the include_all policy so later assertions on team1 policy counts hold. _, err = ds.DeleteTeamPolicies(ctx, team1.ID, []uint{pIncludeAll.ID}) @@ -2530,11 +2553,21 @@ func testPoliciesSave(t *testing.T, ds *Datastore) { assert.NotEqual(t, globalChecksum, globalChecksum2, "Checksum should be different since policy name changed") assert.Equal(t, computeChecksum(*gp), hex.EncodeToString(globalChecksum2)) - // Cannot save a policy with both include and exclude labels + // A policy can be saved with both an include and an exclude scope; both persist. gp2.LabelsExcludeAny = []fleet.LabelIdent{{LabelName: label1.Name}} gp2.LabelsIncludeAny = []fleet.LabelIdent{{LabelName: label2.Name}} err = ds.SavePolicy(ctx, &gp2, false, false) - require.Error(t, err) + require.NoError(t, err) + gpCombined, err := ds.Policy(ctx, gp.ID) + require.NoError(t, err) + requireLabels(t, []string{label2.Name}, gpCombined.LabelsIncludeAny) + requireLabels(t, []string{label1.Name}, gpCombined.LabelsExcludeAny) + + // Restore the exclude-only scope so the remaining assertions are unaffected. + gp2.LabelsExcludeAny = []fleet.LabelIdent{{LabelName: label1.Name}, {LabelName: label2.Name}} + gp2.LabelsIncludeAny = nil + err = ds.SavePolicy(ctx, &gp2, false, false) + require.NoError(t, err) // Change name, query, description and resolution of a team policy. tp2 := *tp1 @@ -6495,14 +6528,23 @@ func testPolicyLabels(t *testing.T, ds *Datastore) { policyIncludeAllOnly1.LabelsIncludeAll = []fleet.LabelIdent{{LabelName: label1.Name}} require.NoError(t, ds.SavePolicy(ctx, policyIncludeAllOnly1, false, false)) + // exclude_all: a host is excluded only when it is a member of ALL the listed labels. + policyExcludeAllBoth := newTestPolicy(t, ds, user1, "policy exclude all both", "", nil) + policyExcludeAllBoth.LabelsExcludeAll = []fleet.LabelIdent{{LabelName: label1.Name}, {LabelName: label2.Name}} + require.NoError(t, ds.SavePolicy(ctx, policyExcludeAllBoth, false, false)) + + policyExcludeAllOnly1 := newTestPolicy(t, ds, user1, "policy exclude all only1", "", nil) + policyExcludeAllOnly1.LabelsExcludeAll = []fleet.LabelIdent{{LabelName: label1.Name}} + require.NoError(t, ds.SavePolicy(ctx, policyExcludeAllOnly1, false, false)) + // The testing grid of truth // - // | hosts \ policies | No labels | include 1 | exclude 2 | include both | exclude both | include_all both | include_all 1 | - // |------------------+-----------+-----------+-----------+--------------+--------------+------------------+---------------| - // | no label | X | | X | | X | | | - // | label 1 | X | X | X | X | | | X | - // | label 2 | X | | | X | | | | - // | label both | X | X | | X | | X | X | + // | hosts \ policies | No labels | include 1 | exclude 2 | include both | exclude both | include_all both | include_all 1 | exclude_all both | exclude_all 1 | + // |------------------+-----------+-----------+-----------+--------------+--------------+------------------+---------------+------------------+---------------| + // | no label | X | | X | | X | | | X | X | + // | label 1 | X | X | X | X | | | X | X | | + // | label 2 | X | | | X | | | | X | X | + // | label both | X | X | | X | | X | X | | | tcs := []struct { Host *fleet.Host @@ -6514,6 +6556,8 @@ func testPolicyLabels(t *testing.T, ds *Datastore) { policyNoLabel, policyExcludeLabel2, policyExcludeBoth, + policyExcludeAllBoth, + policyExcludeAllOnly1, }, }, { @@ -6524,6 +6568,7 @@ func testPolicyLabels(t *testing.T, ds *Datastore) { policyExcludeLabel2, policyIncludeBoth, policyIncludeAllOnly1, + policyExcludeAllBoth, }, }, { @@ -6531,6 +6576,8 @@ func testPolicyLabels(t *testing.T, ds *Datastore) { Policies: []*fleet.Policy{ policyNoLabel, policyIncludeBoth, + policyExcludeAllBoth, + policyExcludeAllOnly1, }, }, { @@ -6626,6 +6673,27 @@ func testPolicyLabelMembershipCleanup(t *testing.T, ds *Datastore) { wantHostsByPol[policy.Name] = []uint{hostNoLabels.ID, hostLabel1.ID} assertPolicyMembership(t, ds, polsByName, wantHostsByPol) + // Re-record membership for all hosts, then switch to exclude_all of both + // labels: a host is removed only if it is a member of BOTH labels. + for _, h := range []*fleet.Host{hostNoLabels, hostLabel1, hostLabel2, hostLabelBoth} { + err = ds.RecordPolicyQueryExecutions(ctx, h, map[uint]*bool{policy.ID: new(true)}, time.Now(), false, nil) + require.NoError(t, err) + } + wantHostsByPol[policy.Name] = []uint{hostNoLabels.ID, hostLabel1.ID, hostLabel2.ID, hostLabelBoth.ID} + assertPolicyMembership(t, ds, polsByName, wantHostsByPol) + + policy.LabelsExcludeAny = nil + policy.LabelsExcludeAll = []fleet.LabelIdent{{LabelName: label1.Name}, {LabelName: label2.Name}} + require.NoError(t, ds.SavePolicy(ctx, policy, false, false)) + + // Only the host that is a member of BOTH labels (hostLabelBoth) is removed. + wantHostsByPol[policy.Name] = []uint{hostNoLabels.ID, hostLabel1.ID, hostLabel2.ID} + assertPolicyMembership(t, ds, polsByName, wantHostsByPol) + + // Reset the policy's label scope so later steps start from a clean slate. + policy.LabelsExcludeAll = nil + require.NoError(t, ds.SavePolicy(ctx, policy, false, false)) + // Test ApplyPolicySpecs with label changes // First, re-record membership for all hosts for _, h := range []*fleet.Host{hostNoLabels, hostLabel1, hostLabel2, hostLabelBoth} { diff --git a/server/fleet/api_policies.go b/server/fleet/api_policies.go index e81cf15b767..82ab1da8560 100644 --- a/server/fleet/api_policies.go +++ b/server/fleet/api_policies.go @@ -15,6 +15,7 @@ type GlobalPolicyRequest struct { LabelsIncludeAny []string `json:"labels_include_any"` LabelsIncludeAll []string `json:"labels_include_all" premium:"true"` LabelsExcludeAny []string `json:"labels_exclude_any"` + LabelsExcludeAll []string `json:"labels_exclude_all" premium:"true"` } type GlobalPolicyResponse struct { @@ -164,6 +165,7 @@ type TeamPolicyRequest struct { LabelsIncludeAny []string `json:"labels_include_any"` LabelsIncludeAll []string `json:"labels_include_all" premium:"true"` LabelsExcludeAny []string `json:"labels_exclude_any"` + LabelsExcludeAll []string `json:"labels_exclude_all" premium:"true"` ConditionalAccessEnabled bool `json:"conditional_access_enabled"` ContinuousAutomationsEnabled bool `json:"continuous_automations_enabled" premium:"true"` Type *string `json:"type"` diff --git a/server/fleet/labels.go b/server/fleet/labels.go index fda08e6af6c..95f0c34fedc 100644 --- a/server/fleet/labels.go +++ b/server/fleet/labels.go @@ -373,6 +373,24 @@ func LabelIdentsToNames(idents []LabelIdent) []string { return out } +// LabelOverlap returns the first label name that appears in both the include +// list and the exclude list, or an empty string if there is none. +// `include` should be the union of all include scopes (e.g. labels_include_all and +// labels_include_any). +// `exclude` should be the union of all exclude scopes. +func LabelOverlap(include, exclude []string) string { + seen := make(map[string]struct{}, len(include)) + for _, n := range include { + seen[n] = struct{}{} + } + for _, n := range exclude { + if _, overlapExists := seen[n]; overlapExists { + return n + } + } + return "" +} + // LabelScope identifies the manner by which labels may be used to scope entities, such as MDM // profiles and software installers, to subsets of hosts. type LabelScope string diff --git a/server/fleet/labels_test.go b/server/fleet/labels_test.go index 1d390e3e573..1954d24a851 100644 --- a/server/fleet/labels_test.go +++ b/server/fleet/labels_test.go @@ -51,3 +51,25 @@ func TestDetectMissingLabels(t *testing.T) { }) } } + +func TestLabelOverlap(t *testing.T) { + testCases := []struct { + name string + include []string + exclude []string + want string + }{ + {name: "no overlap", include: []string{"a", "b"}, exclude: []string{"c", "d"}, want: ""}, + {name: "both empty", include: nil, exclude: nil, want: ""}, + {name: "empty include", include: nil, exclude: []string{"a"}, want: ""}, + {name: "empty exclude", include: []string{"a"}, exclude: nil, want: ""}, + {name: "single overlap", include: []string{"a", "b"}, exclude: []string{"b", "c"}, want: "b"}, + {name: "returns first overlap by exclude order", include: []string{"a", "b", "c"}, exclude: []string{"x", "c", "a"}, want: "c"}, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + assert.Equal(t, tc.want, LabelOverlap(tc.include, tc.exclude)) + }) + } +} diff --git a/server/fleet/mdm.go b/server/fleet/mdm.go index ba755c5008c..e8f2cf62e78 100644 --- a/server/fleet/mdm.go +++ b/server/fleet/mdm.go @@ -1399,7 +1399,7 @@ func ValidateMDMProfileSpecs(invalid *InvalidArgumentError, prefix string, custo fmt.Sprintf(`Couldn't edit %s_settings.configuration_profiles. For each profile, only one of "labels_include_all", "labels_include_any" or "labels" can be included.`, prefix)) } includeLabels := slices.Concat(prof.Labels, prof.LabelsIncludeAll, prof.LabelsIncludeAny) - if overlap := ProfileLabelOverlap(includeLabels, prof.LabelsExcludeAny); overlap != "" { + if overlap := LabelOverlap(includeLabels, prof.LabelsExcludeAny); overlap != "" { invalid.Append(fmt.Sprintf("%s_settings.configuration_profiles", prefix), fmt.Sprintf(`Couldn't edit %s_settings.configuration_profiles. Label %q cannot appear in both include and exclude lists.`, prefix, overlap)) } @@ -1410,22 +1410,6 @@ func ValidateMDMProfileSpecs(invalid *InvalidArgumentError, prefix string, custo } } -// ProfileLabelOverlap returns the first label name that appears in both -// the include list and the exclude list, or an empty string if there is none. -// include should be the union of labels_include_all and labels_include_any. -func ProfileLabelOverlap(include, exclude []string) string { - seen := make(map[string]struct{}, len(include)) - for _, n := range include { - seen[n] = struct{}{} - } - for _, n := range exclude { - if _, overlapExists := seen[n]; overlapExists { - return n - } - } - return "" -} - // GenerateRandom32ByteEntropyURLSafeToken generates a random 32-byte token // and base64 encodes it in a URL safe way (without padding). func GenerateRandom32ByteEntropyURLSafeToken() ([]byte, error) { diff --git a/server/fleet/policies.go b/server/fleet/policies.go index ae5f5222922..93dd0dccba2 100644 --- a/server/fleet/policies.go +++ b/server/fleet/policies.go @@ -2,6 +2,8 @@ package fleet import ( "errors" + "fmt" + "slices" "strings" "time" @@ -54,6 +56,8 @@ type PolicyPayload struct { LabelsIncludeAll []string // LabelsExcludeAny scopes the policy to hosts that are NOT members of ANY of the listed labels. LabelsExcludeAny []string + // LabelsExcludeAll scopes the policy to hosts that are NOT members of ALL of the listed labels. + LabelsExcludeAll []string // ConditionalAccessEnabled indicates whether this is a policy used for Microsoft conditional access. // // Only applies to team policies. @@ -109,6 +113,8 @@ type NewTeamPolicyPayload struct { LabelsIncludeAll []string // LabelsExcludeAny scopes the policy to hosts that are NOT members of ANY of the listed labels. LabelsExcludeAny []string + // LabelsExcludeAll scopes the policy to hosts that are NOT members of ALL of the listed labels. + LabelsExcludeAll []string // ConditionalAccessEnabled indicates whether this is a policy used for Microsoft conditional access. ConditionalAccessEnabled bool @@ -126,7 +132,8 @@ var ( errPolicyEmptyQuery = errors.New("policy query cannot be empty") errPolicyIDAndQuerySet = errors.New("both fields \"queryID\" and \"query\" cannot be set") errPolicyInvalidPlatform = errors.New("invalid policy platform") - ErrPolicyConflictingLabels = errors.New("policy can include at most one of labels_include_any, labels_include_all, or labels_exclude_any") + ErrPolicyConflictingIncludeLabels = errors.New("policy can include at most one of labels_include_any or labels_include_all") + ErrPolicyConflictingExcludeLabels = errors.New("policy can include at most one of labels_exclude_any or labels_exclude_all") errPolicyPatchAndQuerySet = errors.New("If the \"type\" is \"patch\", the \"query\" field is not supported.") errPolicyPatchAndPlatformSet = errors.New("If the \"type\" is \"patch\", the \"platform\" field is not supported.") errPolicyPatchNoTitleID = errors.New("If the \"type\" is \"patch\", the \"patch_software_title_id\" field is required.") @@ -157,7 +164,7 @@ func (p PolicyPayload) Verify() error { if p.PatchSoftwareTitleID == nil { return errPolicyPatchNoTitleID } - if err := verifyLabelScopeMutualExclusion(p.LabelsIncludeAny, p.LabelsIncludeAll, p.LabelsExcludeAny); err != nil { + if err := VerifyPolicyLabelScopes(p.LabelsIncludeAny, p.LabelsIncludeAll, p.LabelsExcludeAny, p.LabelsExcludeAll); err != nil { return err } return nil @@ -182,25 +189,42 @@ func (p PolicyPayload) Verify() error { return err } - return verifyLabelScopeMutualExclusion(p.LabelsIncludeAny, p.LabelsIncludeAll, p.LabelsExcludeAny) + return VerifyPolicyLabelScopes(p.LabelsIncludeAny, p.LabelsIncludeAll, p.LabelsExcludeAny, p.LabelsExcludeAll) } -// verifyLabelScopeMutualExclusion enforces that at most one of the three label -// scope slices carries values. Empty slices ([]) are treated as "no value" and -// are ignored, so e.g. {LabelsIncludeAny: [], LabelsIncludeAll: [A]} is valid. -func verifyLabelScopeMutualExclusion(includeAny, includeAll, excludeAny []string) error { - specified := 0 +// VerifyPolicyLabelScopes enforces the policy label-targeting rules: at most one +// include scope (labels_include_any or labels_include_all) and at most one +// exclude scope (labels_exclude_any or labels_exclude_all) may carry values, and +// no label may appear in both an include and an exclude list. An include scope +// and an exclude scope may be combined. Empty slices ([]) are treated as "no +// value", so e.g. {LabelsIncludeAny: [], LabelsIncludeAll: [A]} is valid. +func VerifyPolicyLabelScopes(includeAny, includeAll, excludeAny, excludeAll []string) error { + includeScopes := 0 if len(includeAny) > 0 { - specified++ + includeScopes++ } if len(includeAll) > 0 { - specified++ + includeScopes++ } + if includeScopes > 1 { + return ErrPolicyConflictingIncludeLabels + } + + excludeScopes := 0 if len(excludeAny) > 0 { - specified++ + excludeScopes++ + } + if len(excludeAll) > 0 { + excludeScopes++ } - if specified > 1 { - return ErrPolicyConflictingLabels + if excludeScopes > 1 { + return ErrPolicyConflictingExcludeLabels + } + + include := slices.Concat(includeAny, includeAll) + exclude := slices.Concat(excludeAny, excludeAll) + if overlap := LabelOverlap(include, exclude); overlap != "" { + return fmt.Errorf("label %q cannot appear in both an include and an exclude list", overlap) } return nil } @@ -287,6 +311,8 @@ type ModifyPolicyPayload struct { LabelsIncludeAll []string `json:"labels_include_all" premium:"true"` // LabelsExcludeAny scopes the policy to hosts that are NOT members of ANY of the listed labels. LabelsExcludeAny []string `json:"labels_exclude_any"` + // LabelsExcludeAll scopes the policy to hosts that are NOT members of ALL of the listed labels. + LabelsExcludeAll []string `json:"labels_exclude_all" premium:"true"` // ConditionalAccessEnabled indicates whether this is a policy used for Microsoft conditional access. // // Only applies to team policies. @@ -315,7 +341,7 @@ func (p ModifyPolicyPayload) Verify() error { if p.Platform != nil { return errPolicyPlatformUpdated } - return verifyLabelScopeMutualExclusion(p.LabelsIncludeAny, p.LabelsIncludeAll, p.LabelsExcludeAny) + return VerifyPolicyLabelScopes(p.LabelsIncludeAny, p.LabelsIncludeAll, p.LabelsExcludeAny, p.LabelsExcludeAll) } if p.Name != nil { @@ -333,7 +359,7 @@ func (p ModifyPolicyPayload) Verify() error { return err } } - return verifyLabelScopeMutualExclusion(p.LabelsIncludeAny, p.LabelsIncludeAll, p.LabelsExcludeAny) + return VerifyPolicyLabelScopes(p.LabelsIncludeAny, p.LabelsIncludeAll, p.LabelsExcludeAny, p.LabelsExcludeAll) } // PolicyData holds data of a fleet policy. @@ -372,6 +398,8 @@ type PolicyData struct { LabelsIncludeAll []LabelIdent `json:"labels_include_all,omitempty"` // LabelsExcludeAny scopes the policy to hosts that are NOT members of ANY of the listed labels. LabelsExcludeAny []LabelIdent `json:"labels_exclude_any,omitempty"` + // LabelsExcludeAll scopes the policy to hosts that are NOT members of ALL of the listed labels. + LabelsExcludeAll []LabelIdent `json:"labels_exclude_all,omitempty"` // CalendarEventsEnabled indicates whether calendar events are enabled for the policy. // @@ -523,6 +551,7 @@ type PolicySpec struct { LabelsIncludeAny []string `json:"labels_include_any,omitempty"` LabelsIncludeAll []string `json:"labels_include_all,omitempty"` LabelsExcludeAny []string `json:"labels_exclude_any,omitempty"` + LabelsExcludeAll []string `json:"labels_exclude_all,omitempty"` // ConditionalAccessEnabled indicates whether this is a policy used for Microsoft conditional access. // // Only applies to team policies. @@ -578,7 +607,7 @@ func (p PolicySpec) Verify() error { if err := verifyPatchPolicy(p.Team, p.Type); err != nil { return err } - return verifyLabelScopeMutualExclusion(p.LabelsIncludeAny, p.LabelsIncludeAll, p.LabelsExcludeAny) + return VerifyPolicyLabelScopes(p.LabelsIncludeAny, p.LabelsIncludeAll, p.LabelsExcludeAny, p.LabelsExcludeAll) } // FirstDuplicatePolicySpecName returns first duplicate name of policies (in a team) or empty string if no duplicates found diff --git a/server/fleet/policies_test.go b/server/fleet/policies_test.go index bc3d51a1803..6ec38f0e3c3 100644 --- a/server/fleet/policies_test.go +++ b/server/fleet/policies_test.go @@ -27,6 +27,48 @@ func TestVerifyPolicyPlatforms(t *testing.T) { } } +func TestVerifyPolicyLabelScopes(t *testing.T) { + testCases := []struct { + name string + includeAny []string + includeAll []string + excludeAny []string + excludeAll []string + wantErr error // sentinel to match with errors.Is (nil means no error) + wantErrContains string // substring to match for the dynamic overlap error + }{ + {name: "no labels"}, + {name: "include_any only", includeAny: []string{"a"}}, + {name: "include_all only", includeAll: []string{"a"}}, + {name: "exclude_any only", excludeAny: []string{"a"}}, + {name: "exclude_all only", excludeAll: []string{"a"}}, + {name: "include_any + exclude_any combined", includeAny: []string{"a"}, excludeAny: []string{"b"}}, + {name: "include_all + exclude_any combined", includeAll: []string{"a"}, excludeAny: []string{"b"}}, + {name: "include_any + exclude_all combined", includeAny: []string{"a"}, excludeAll: []string{"b"}}, + {name: "include_all + exclude_all combined", includeAll: []string{"a"}, excludeAll: []string{"b"}}, + {name: "include_any + include_all conflict", includeAny: []string{"a"}, includeAll: []string{"b"}, wantErr: ErrPolicyConflictingIncludeLabels}, + {name: "exclude_any + exclude_all conflict", excludeAny: []string{"a"}, excludeAll: []string{"b"}, wantErr: ErrPolicyConflictingExcludeLabels}, + {name: "overlap include_any/exclude_any", includeAny: []string{"a"}, excludeAny: []string{"a"}, wantErrContains: `label "a" cannot appear in both an include and an exclude list`}, + {name: "overlap include_all/exclude_all", includeAll: []string{"a"}, excludeAll: []string{"a"}, wantErrContains: `label "a" cannot appear in both an include and an exclude list`}, + {name: "overlap include_any/exclude_all", includeAny: []string{"a"}, excludeAll: []string{"a"}, wantErrContains: `label "a" cannot appear in both an include and an exclude list`}, + {name: "overlap include_all/exclude_any", includeAll: []string{"a"}, excludeAny: []string{"a"}, wantErrContains: `label "a" cannot appear in both an include and an exclude list`}, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + err := VerifyPolicyLabelScopes(tc.includeAny, tc.includeAll, tc.excludeAny, tc.excludeAll) + switch { + case tc.wantErrContains != "": + require.ErrorContains(t, err, tc.wantErrContains) + case tc.wantErr != nil: + require.ErrorIs(t, err, tc.wantErr) + default: + require.NoError(t, err) + } + }) + } +} + func TestFirstFuplicatePolicySpecName(t *testing.T) { testCases := []struct { name string diff --git a/server/service/apple_mdm.go b/server/service/apple_mdm.go index b3e4856f999..51350f11776 100644 --- a/server/service/apple_mdm.go +++ b/server/service/apple_mdm.go @@ -441,7 +441,7 @@ func (svc *Service) NewMDMAppleConfigProfile(ctx context.Context, teamID uint, d cp.Mobileconfig = data cp.SecretsUpdatedAt = secretsUpdatedAt - if overlap := fleet.ProfileLabelOverlap(labelsInclude, labelsExcludeAny); overlap != "" { + if overlap := fleet.LabelOverlap(labelsInclude, labelsExcludeAny); overlap != "" { return nil, ctxerr.Wrap(ctx, fleet.NewInvalidArgumentError("labels", fmt.Sprintf("label %q cannot appear in both include and exclude lists", overlap))) } includeLabels, excludeLabels, err := svc.validateProfileLabelSets(ctx, &teamID, labelsInclude, labelsExcludeAny) @@ -896,7 +896,7 @@ func (svc *Service) NewMDMAppleDeclaration(ctx context.Context, teamID uint, dat } } - if overlap := fleet.ProfileLabelOverlap(labelsInclude, labelsExcludeAny); overlap != "" { + if overlap := fleet.LabelOverlap(labelsInclude, labelsExcludeAny); overlap != "" { return nil, ctxerr.Wrap(ctx, fleet.NewInvalidArgumentError("labels", fmt.Sprintf("label %q cannot appear in both include and exclude lists", overlap))) } validatedIncludeLabels, excludeLabels, err := svc.validateDeclarationLabelSets(ctx, teamID, labelsInclude, labelsExcludeAny) diff --git a/server/service/global_policies.go b/server/service/global_policies.go index 6ce37d85de9..27a52d4faa3 100644 --- a/server/service/global_policies.go +++ b/server/service/global_policies.go @@ -38,6 +38,7 @@ func globalPolicyEndpoint(ctx context.Context, request interface{}, svc fleet.Se LabelsIncludeAny: req.LabelsIncludeAny, LabelsIncludeAll: req.LabelsIncludeAll, LabelsExcludeAny: req.LabelsExcludeAny, + LabelsExcludeAll: req.LabelsExcludeAll, Type: fleet.PolicyTypeDynamic, }) if err != nil { @@ -61,11 +62,11 @@ func (svc Service) NewGlobalPolicy(ctx context.Context, p fleet.PolicyPayload) ( }) } - if len(p.LabelsIncludeAll) > 0 && !license.IsPremium(ctx) { + if (len(p.LabelsIncludeAll) > 0 || len(p.LabelsExcludeAll) > 0) && !license.IsPremium(ctx) { return nil, fleet.ErrMissingLicense } - if err := verifyLabelsToAssociate(ctx, svc.ds, nil, slices.Concat(p.LabelsIncludeAny, p.LabelsIncludeAll, p.LabelsExcludeAny), vc.User); err != nil { + if err := verifyLabelsToAssociate(ctx, svc.ds, nil, slices.Concat(p.LabelsIncludeAny, p.LabelsIncludeAll, p.LabelsExcludeAny, p.LabelsExcludeAll), vc.User); err != nil { return nil, ctxerr.Wrap(ctx, err, "verify labels to associate") } @@ -471,8 +472,8 @@ func (svc *Service) ApplyPolicySpecs(ctx context.Context, policies []*fleet.Poli }) } - // LabelsIncludeAll is premium-only. - if len(policy.LabelsIncludeAll) > 0 && !license.IsPremium(ctx) { + // LabelsIncludeAll and LabelsExcludeAll are premium-only. + if (len(policy.LabelsIncludeAll) > 0 || len(policy.LabelsExcludeAll) > 0) && !license.IsPremium(ctx) { return fleet.ErrMissingLicense } @@ -482,7 +483,7 @@ func (svc *Service) ApplyPolicySpecs(ctx context.Context, policies []*fleet.Poli } // Make sure any applied labels exist. - labels := slices.Concat(policy.LabelsIncludeAny, policy.LabelsIncludeAll, policy.LabelsExcludeAny) + labels := slices.Concat(policy.LabelsIncludeAny, policy.LabelsIncludeAll, policy.LabelsExcludeAny, policy.LabelsExcludeAll) if len(labels) > 0 { var teamID *uint // ensure labels specified exist and are global or on the same team as the policy if policy.Team != "" { // if we get 0 as team ID, we'll pull only global labels, which is fine diff --git a/server/service/integration_enterprise_test.go b/server/service/integration_enterprise_test.go index df83f2e4eb5..dd1d00bbce0 100644 --- a/server/service/integration_enterprise_test.go +++ b/server/service/integration_enterprise_test.go @@ -29722,8 +29722,9 @@ func (s *integrationEnterpriseTestSuite) TestAPIOnlyGitOpsUserWithEndpointRestri } // TestPolicyLabelsIncludeAll exercises the full create/modify/spec stack for the -// new include_all label scope on policies, including strict mutex rejection -// at every API entry point and end-to-end host targeting. +// include_all label scope on policies: combining one include scope with one +// exclude scope, rejecting two include or two exclude scopes at every API entry +// point, and end-to-end host targeting. func (s *integrationEnterpriseTestSuite) TestPolicyLabelsIncludeAll() { t := s.T() ctx := context.Background() @@ -29779,16 +29780,28 @@ func (s *integrationEnterpriseTestSuite) TestPolicyLabelsIncludeAll() { LabelsIncludeAll: []string{lblA.Name}, LabelsIncludeAny: []string{lblB.Name}, }, http.StatusBadRequest) - require.Contains(t, extractServerErrorText(rej1Resp.Body), fleet.ErrPolicyConflictingLabels.Error()) + require.Contains(t, extractServerErrorText(rej1Resp.Body), fleet.ErrPolicyConflictingIncludeLabels.Error()) - // Mutex rejection on POST: include_all + exclude_any. - rej2Resp := s.Do("POST", "/api/latest/fleet/policies", fleet.GlobalPolicyRequest{ - Name: "rej2-" + t.Name(), + // include_all + exclude_any is now a valid combination (one include scope + // plus one exclude scope), so it succeeds and persists both. + var combinedResp fleet.GlobalPolicyResponse + s.DoJSON("POST", "/api/latest/fleet/policies", fleet.GlobalPolicyRequest{ + Name: "combined-" + t.Name(), Query: "SELECT 1", LabelsIncludeAll: []string{lblA.Name}, LabelsExcludeAny: []string{lblB.Name}, + }, http.StatusOK, &combinedResp) + require.Len(t, combinedResp.Policy.LabelsIncludeAll, 1) + require.Len(t, combinedResp.Policy.LabelsExcludeAny, 1) + + // Two exclude scopes (exclude_any + exclude_all) are still rejected. + rejExclResp := s.Do("POST", "/api/latest/fleet/policies", fleet.GlobalPolicyRequest{ + Name: "rej-excl-" + t.Name(), + Query: "SELECT 1", + LabelsExcludeAny: []string{lblA.Name}, + LabelsExcludeAll: []string{lblB.Name}, }, http.StatusBadRequest) - require.Contains(t, extractServerErrorText(rej2Resp.Body), fleet.ErrPolicyConflictingLabels.Error()) + require.Contains(t, extractServerErrorText(rejExclResp.Body), fleet.ErrPolicyConflictingExcludeLabels.Error()) // 3. PATCH: switch existing include_all policy to include_any. Other slices should clear. switchAny := []string{lblA.Name} @@ -29817,7 +29830,7 @@ func (s *integrationEnterpriseTestSuite) TestPolicyLabelsIncludeAll() { LabelsIncludeAny: []string{lblB.Name}, }, }, http.StatusBadRequest) - require.Contains(t, extractServerErrorText(rejPatchResp.Body), fleet.ErrPolicyConflictingLabels.Error()) + require.Contains(t, extractServerErrorText(rejPatchResp.Body), fleet.ErrPolicyConflictingIncludeLabels.Error()) // 4. End-to-end host targeting: only hostBoth should match the include_all policy. policy, err := s.ds.Policy(ctx, createResp.Policy.ID) @@ -30491,28 +30504,28 @@ func (s *integrationEnterpriseTestSuite) TestApplyPolicySpecsBatchMixedScopes() } } - // Batch with the mutex-violating spec at the END — proves we don't persist + // Batch with an invalid (two-include) spec at the END — proves we don't persist // preceding valid specs once a later one fails validation. rejResp := s.Do("POST", "/api/latest/fleet/spec/policies", fleet.ApplyPolicySpecsRequest{ Specs: []*fleet.PolicySpec{ {Name: validAnyName, Query: "SELECT 1", LabelsIncludeAny: []string{lblA.Name}}, {Name: validAllName, Query: "SELECT 1", LabelsIncludeAll: []string{lblA.Name, lblB.Name}}, - {Name: invalidName, Query: "SELECT 1", LabelsIncludeAll: []string{lblA.Name}, LabelsExcludeAny: []string{lblB.Name}}, + {Name: invalidName, Query: "SELECT 1", LabelsIncludeAll: []string{lblA.Name}, LabelsIncludeAny: []string{lblB.Name}}, }, }, http.StatusBadRequest) - require.Contains(t, extractServerErrorText(rejResp.Body), fleet.ErrPolicyConflictingLabels.Error()) + require.Contains(t, extractServerErrorText(rejResp.Body), fleet.ErrPolicyConflictingIncludeLabels.Error()) assertNonePersisted("violator-last") - // Batch with the mutex-violating spec at the FRONT — proves we don't persist + // Batch with an invalid (two-include) spec at the FRONT — proves we don't persist // trailing valid specs after a per-spec validation pass that the loop never reaches. rejResp = s.Do("POST", "/api/latest/fleet/spec/policies", fleet.ApplyPolicySpecsRequest{ Specs: []*fleet.PolicySpec{ - {Name: invalidName, Query: "SELECT 1", LabelsIncludeAll: []string{lblA.Name}, LabelsExcludeAny: []string{lblB.Name}}, + {Name: invalidName, Query: "SELECT 1", LabelsIncludeAll: []string{lblA.Name}, LabelsIncludeAny: []string{lblB.Name}}, {Name: validAnyName, Query: "SELECT 1", LabelsIncludeAny: []string{lblA.Name}}, {Name: validAllName, Query: "SELECT 1", LabelsIncludeAll: []string{lblA.Name, lblB.Name}}, }, }, http.StatusBadRequest) - require.Contains(t, extractServerErrorText(rejResp.Body), fleet.ErrPolicyConflictingLabels.Error()) + require.Contains(t, extractServerErrorText(rejResp.Body), fleet.ErrPolicyConflictingIncludeLabels.Error()) assertNonePersisted("violator-first") // Fully-valid 3-spec batch (one per scope) succeeds. diff --git a/server/service/mdm.go b/server/service/mdm.go index 5be8c5295f8..ecab6cedfd5 100644 --- a/server/service/mdm.go +++ b/server/service/mdm.go @@ -1701,7 +1701,7 @@ func (newMDMConfigProfileRequest) DecodeRequest(ctx context.Context, r *http.Req } includeLabels := append(decoded.LabelsIncludeAll, decoded.LabelsIncludeAny...) //nolint:gocritic - if overlap := fleet.ProfileLabelOverlap(includeLabels, decoded.LabelsExcludeAny); overlap != "" { + if overlap := fleet.LabelOverlap(includeLabels, decoded.LabelsExcludeAny); overlap != "" { return nil, &fleet.BadRequestError{Message: fmt.Sprintf(`Label %q cannot appear in both include and exclude lists.`, overlap)} } @@ -1881,7 +1881,7 @@ func (svc *Service) NewMDMAndroidConfigProfile(ctx context.Context, teamID uint, return nil, ctxerr.Wrap(ctx, err, "validate profile") } - if overlap := fleet.ProfileLabelOverlap(labelsInclude, labelsExcludeAny); overlap != "" { + if overlap := fleet.LabelOverlap(labelsInclude, labelsExcludeAny); overlap != "" { return nil, ctxerr.Wrap(ctx, fleet.NewInvalidArgumentError("labels", fmt.Sprintf("label %q cannot appear in both include and exclude lists", overlap))) } includeLabels, excludeLabels, err := svc.validateProfileLabelSets(ctx, &teamID, labelsInclude, labelsExcludeAny) @@ -2840,7 +2840,7 @@ func validateProfiles(profiles map[int]fleet.MDMProfileBatchPayload) error { } includeLabels := append(profile.LabelsIncludeAll, append(profile.Labels, profile.LabelsIncludeAny...)...) //nolint:gocritic - if overlap := fleet.ProfileLabelOverlap(includeLabels, profile.LabelsExcludeAny); overlap != "" { + if overlap := fleet.LabelOverlap(includeLabels, profile.LabelsExcludeAny); overlap != "" { return fleet.NewInvalidArgumentError("mdm", fmt.Sprintf(`Couldn't edit configuration_profiles. Label %q cannot appear in both include and exclude lists.`, overlap)) } diff --git a/server/service/team_policies.go b/server/service/team_policies.go index a59c5c5bad4..1a44bbd0433 100644 --- a/server/service/team_policies.go +++ b/server/service/team_policies.go @@ -38,6 +38,7 @@ func teamPolicyEndpoint(ctx context.Context, request interface{}, svc fleet.Serv LabelsIncludeAny: req.LabelsIncludeAny, LabelsIncludeAll: req.LabelsIncludeAll, LabelsExcludeAny: req.LabelsExcludeAny, + LabelsExcludeAll: req.LabelsExcludeAll, ConditionalAccessEnabled: req.ConditionalAccessEnabled, ContinuousAutomationsEnabled: req.ContinuousAutomationsEnabled, Type: req.Type, @@ -74,11 +75,11 @@ func (svc Service) NewTeamPolicy(ctx context.Context, teamID uint, tp fleet.NewT }) } - if len(tp.LabelsIncludeAll) > 0 && !license.IsPremium(ctx) { + if (len(tp.LabelsIncludeAll) > 0 || len(tp.LabelsExcludeAll) > 0) && !license.IsPremium(ctx) { return nil, fleet.ErrMissingLicense } - if err := verifyLabelsToAssociate(ctx, svc.ds, &teamID, slices.Concat(tp.LabelsIncludeAny, tp.LabelsIncludeAll, tp.LabelsExcludeAny), vc.User); err != nil { + if err := verifyLabelsToAssociate(ctx, svc.ds, &teamID, slices.Concat(tp.LabelsIncludeAny, tp.LabelsIncludeAll, tp.LabelsExcludeAny, tp.LabelsExcludeAll), vc.User); err != nil { return nil, ctxerr.Wrap(ctx, err, "verify labels to associate") } @@ -301,6 +302,7 @@ func (svc *Service) newTeamPolicyPayloadToPolicyPayload(ctx context.Context, tea LabelsIncludeAny: p.LabelsIncludeAny, LabelsIncludeAll: p.LabelsIncludeAll, LabelsExcludeAny: p.LabelsExcludeAny, + LabelsExcludeAll: p.LabelsExcludeAll, ConditionalAccessEnabled: p.ConditionalAccessEnabled, ContinuousAutomationsEnabled: p.ContinuousAutomationsEnabled, Type: policyType, @@ -626,11 +628,11 @@ func (svc *Service) modifyPolicy(ctx context.Context, teamID *uint, id uint, p f }) } - if len(p.LabelsIncludeAll) > 0 && !license.IsPremium(ctx) { + if (len(p.LabelsIncludeAll) > 0 || len(p.LabelsExcludeAll) > 0) && !license.IsPremium(ctx) { return nil, fleet.ErrMissingLicense } - if err := verifyLabelsToAssociate(ctx, svc.ds, teamID, slices.Concat(p.LabelsIncludeAny, p.LabelsIncludeAll, p.LabelsExcludeAny), authz.UserFromContext(ctx)); err != nil { + if err := verifyLabelsToAssociate(ctx, svc.ds, teamID, slices.Concat(p.LabelsIncludeAny, p.LabelsIncludeAll, p.LabelsExcludeAny, p.LabelsExcludeAll), authz.UserFromContext(ctx)); err != nil { return nil, ctxerr.Wrap(ctx, err, "verify labels to associate") } @@ -705,14 +707,16 @@ func (svc *Service) modifyPolicy(ctx context.Context, teamID *uint, id uint, p f policy.ScriptID = &p.ScriptID.Value } } - // If the client sent any of the three label scope fields, treat all three as authoritative - // for the policy's label state. The validator on ModifyPolicyPayload (Verify()) enforces that at most - // one is non-nil, so a single field switches scope and clears the others. Sending none of - // the three leaves labels untouched. - if p.LabelsIncludeAny != nil || p.LabelsIncludeAll != nil || p.LabelsExcludeAny != nil { + // If the client sent any of the label scope fields, treat all of them as authoritative + // for the policy's label state. Verify() enforces that at most one include scope and one + // exclude scope carry values (empty slices are allowed and just clear that scope), so the + // provided fields switch scope and clear the others. Sending none of them (all nil) leaves + // labels untouched. + if p.LabelsIncludeAny != nil || p.LabelsIncludeAll != nil || p.LabelsExcludeAny != nil || p.LabelsExcludeAll != nil { policy.LabelsIncludeAny = fleet.LabelNamesToIdents(p.LabelsIncludeAny) policy.LabelsIncludeAll = fleet.LabelNamesToIdents(p.LabelsIncludeAll) policy.LabelsExcludeAny = fleet.LabelNamesToIdents(p.LabelsExcludeAny) + policy.LabelsExcludeAll = fleet.LabelNamesToIdents(p.LabelsExcludeAll) } if err := fleet.PolicyVerifyConditionalAccess(policy.ConditionalAccessEnabled, policy.Platform); err != nil { diff --git a/server/service/windows_mdm_profiles.go b/server/service/windows_mdm_profiles.go index 70aee9e29e7..e79632c7886 100644 --- a/server/service/windows_mdm_profiles.go +++ b/server/service/windows_mdm_profiles.go @@ -76,7 +76,7 @@ func (svc *Service) NewMDMWindowsConfigProfile(ctx context.Context, teamID uint, return nil, ctxerr.Wrap(ctx, err, "validate profile") } - if overlap := fleet.ProfileLabelOverlap(labelsInclude, labelsExcludeAny); overlap != "" { + if overlap := fleet.LabelOverlap(labelsInclude, labelsExcludeAny); overlap != "" { return nil, ctxerr.Wrap(ctx, fleet.NewInvalidArgumentError("labels", fmt.Sprintf("label %q cannot appear in both include and exclude lists", overlap))) } includeLabels, excludeLabels, err := svc.validateProfileLabelSets(ctx, &teamID, labelsInclude, labelsExcludeAny)