Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion server/datastore/mysql/policies.go
Original file line number Diff line number Diff line change
Expand Up @@ -1503,12 +1503,20 @@ func (ds *Datastore) ApplyPolicySpecs(ctx context.Context, authorID uint, specs
spec.Query = generated.Query
}

// patch_software_title_id is only for type=patch. Dynamic policies may still
// carry fleet_maintained_app_slug (e.g. install_software); they must not
// reuse the same title id or they collide on idx_team_id_patch_software_title_id.
var patchSoftwareTitleIDArg *uint
if spec.Type == fleet.PolicyTypePatch {
patchSoftwareTitleIDArg = fmaTitleID
}

res, err := tx.ExecContext(
ctx,
query,
spec.Name, spec.Query, spec.Description, authorID, spec.Resolution, teamID, spec.Platform, spec.Critical,
spec.CalendarEventsEnabled, softwareInstallerID, vppAppsTeamsID, scriptID, spec.ConditionalAccessEnabled,
spec.Type, fmaTitleID,
spec.Type, patchSoftwareTitleIDArg,
)
Comment thread
allenhouchins marked this conversation as resolved.
if err != nil {
return ctxerr.Wrap(ctx, err, "exec ApplyPolicySpecs insert")
Expand Down
100 changes: 100 additions & 0 deletions server/datastore/mysql/policies_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ func TestPolicies(t *testing.T) {
{"ResetAttemptsOnFailingToPassingAsync", testResetAttemptsOnFailingToPassingAsync},
{"PolicyModificationResetsAttemptNumber", testPolicyModificationResetsAttemptNumber},
{"TeamPatchPolicy", testTeamPatchPolicy},
{"ApplyPolicySpecsDynamicAndPatchSameFMA", testApplyPolicySpecsDynamicAndPatchSameFMA},
{"TeamPolicyAutomationFilter", testTeamPolicyAutomationFilter},
{"BatchedPolicyMembershipCleanup", testBatchedPolicyMembershipCleanup},
{"BatchedPolicyMembershipCleanupOnPolicyUpdate", testBatchedPolicyMembershipCleanupOnPolicyUpdate},
Expand Down Expand Up @@ -7824,6 +7825,105 @@ func testTeamPatchPolicy(t *testing.T, ds *Datastore) {
require.NotEqual(t, previousChecksum, newChecksum)
}

// testApplyPolicySpecsDynamicAndPatchSameFMA is a regression test for a unique
// index collision on (team_id, patch_software_title_id). A dynamic policy that
// carries fleet_maintained_app_slug (e.g. install_software automation) must not
// reuse the FMA's title id in patch_software_title_id, otherwise a separate
// patch policy on the same team and FMA collides with it.
func testApplyPolicySpecsDynamicAndPatchSameFMA(t *testing.T, ds *Datastore) {
ctx := context.Background()
user1 := test.NewUser(t, ds, "Alice", "alice@example.com", true)
team1, err := ds.NewTeam(ctx, &fleet.Team{Name: "team-fma-collision"})
require.NoError(t, err)

maintainedApp, err := ds.UpsertMaintainedApp(ctx, &fleet.MaintainedApp{
Name: "Maintained1",
Slug: "maintained1",
Platform: "darwin",
UniqueIdentifier: "fleet.maintained1",
})
require.NoError(t, err)

_, fmaTitleID, err := ds.MatchOrCreateSoftwareInstaller(ctx, &fleet.UploadSoftwareInstallerPayload{
InstallScript: "hello",
PreInstallQuery: "SELECT 1",
PostInstallScript: "world",
StorageID: "storage-fma-collision",
Filename: "maintained1",
Title: "Maintained1",
Version: "1.0",
Source: "apps",
Platform: "darwin",
BundleIdentifier: "fleet.maintained1",
UserID: user1.ID,
TeamID: &team1.ID,
ValidatedLabels: &fleet.LabelIdentsWithScope{},
FleetMaintainedAppID: &maintainedApp.ID,
})
require.NoError(t, err)

// Apply a dynamic install_software policy that references the FMA slug. With
// the bug, the FMA's title id was written into patch_software_title_id even
// though spec.Type was dynamic.
err = ds.ApplyPolicySpecs(ctx, user1.ID, []*fleet.PolicySpec{
{
Name: "install-fma-dynamic",
Query: "SELECT 1;",
Team: team1.Name,
Platform: "darwin",
Type: fleet.PolicyTypeDynamic,
FleetMaintainedAppSlug: "maintained1",
SoftwareTitleID: &fmaTitleID,
},
})
require.NoError(t, err)

// Apply a patch policy on the same team for the same FMA slug. With the bug
// this collided with the dynamic row on (team_id, patch_software_title_id).
err = ds.ApplyPolicySpecs(ctx, user1.ID, []*fleet.PolicySpec{
{
Name: "patch-fma",
Query: "SELECT 1;",
Team: team1.Name,
Type: fleet.PolicyTypePatch,
FleetMaintainedAppSlug: "maintained1",
},
})
require.NoError(t, err)

// Verify both policies exist and that patch_software_title_id is only set on
// the patch policy.
type policyRow struct {
Name string `db:"name"`
Type string `db:"type"`
PatchSoftwareTitleID *uint `db:"patch_software_title_id"`
}
var rows []policyRow
ExecAdhocSQL(t, ds, func(q sqlx.ExtContext) error {
return sqlx.SelectContext(ctx, q, &rows,
`SELECT name, type, patch_software_title_id FROM policies WHERE team_id = ? ORDER BY type`,
team1.ID,
)
})
require.Len(t, rows, 2)

byType := make(map[string]policyRow, len(rows))
for _, r := range rows {
byType[r.Type] = r
}

dyn, ok := byType[fleet.PolicyTypeDynamic]
require.True(t, ok, "dynamic policy must be persisted")
require.Equal(t, "install-fma-dynamic", dyn.Name)
require.Nil(t, dyn.PatchSoftwareTitleID, "dynamic policy must not set patch_software_title_id")

patch, ok := byType[fleet.PolicyTypePatch]
require.True(t, ok, "patch policy must be persisted")
require.Equal(t, "patch-fma", patch.Name)
require.NotNil(t, patch.PatchSoftwareTitleID, "patch policy must set patch_software_title_id")
require.Equal(t, fmaTitleID, *patch.PatchSoftwareTitleID)
}

func testTeamPolicyAutomationFilter(t *testing.T, ds *Datastore) {
ctx := context.Background()
user1 := test.NewUser(t, ds, "Alice", "alice@example.com", true)
Expand Down
Loading