Patch when closed: policy and package api (#49871)

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

- Adds patch_when_closed to the new policy, update policy, and update
package endpoints
- Sets pre_install_query in the software title endpoint to
software_installers.app_open_query if the policy is a patch when closed
policy (discussed in standup)
- Deletes the existing pre_install_query if setting patch_when_closed
(discussed in standup)
- Fixes a small error in the patch_policy package
- Fixes the default macOS query in patch_policy package to use to escape
symbols in a.path


# Checklist for submitter

If some of the following don't apply, delete the relevant line.

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

- [ ] 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.
- [ ] Timeouts are implemented and retries are limited to avoid infinite
loops
- [ ] If paths of existing endpoints are modified without backwards
compatibility, checked the frontend/CLI for any necessary changes

## Testing

- [x] Added/updated automated tests
- [ ] Where appropriate, [automated tests simulate multiple hosts and
test for host
isolation](https://github.com/fleetdm/fleet/blob/main/docs/Contributing/reference/patterns-backend.md#unit-testing)
(updates to one hosts's records do not affect another)

- [x] QA'd all new/changed functionality manually


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **New Features**
* Added a “patch when closed” option for patch policies and
Fleet-maintained apps.
* Added installer controls for enabling patching and configuring whether
apps must be closed before installation.
* Automatically manages pre-install behavior and continuous automation
requirements for these policies.

* **Bug Fixes**
  * Improved app detection based on application path prefixes on macOS.
  * Corrected RStudio process detection on Windows.
  * Added validation to prevent incompatible patch policy settings.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
Jonathan Katz
2026-07-30 11:14:20 -04:00
committed by GitHub
parent 20fdf5dcd3
commit 345eb0538b
18 changed files with 345 additions and 162 deletions
@@ -216,7 +216,7 @@ func TestIngestValidations(t *testing.T) {
// The managed "is app open" query matches a running process inside the app bundle.
require.Equal(t,
fmt.Sprintf("SELECT 1 WHERE NOT EXISTS (SELECT 1 FROM apps a JOIN processes p ON p.path LIKE concat(a.path, '/%%') WHERE a.bundle_identifier = '%s');", out.UniqueIdentifier),
fmt.Sprintf("SELECT 1 WHERE NOT EXISTS (SELECT 1 FROM apps a JOIN processes p ON substr(p.path, 1, LENGTH(a.path) + 1) = concat(a.path, '/') WHERE a.bundle_identifier = '%s');", out.UniqueIdentifier),
out.Queries.Open,
)
})
+68 -57
View File
@@ -809,8 +809,18 @@ func (svc *Service) UpdateSoftwareInstaller(ctx context.Context, payload *fleet.
}
var shouldDoSideEffects bool
if err := svc.reconcilePatchPolicy(ctx, payload, existingInstaller); err != nil {
return nil, err
var existingPolicy *fleet.PatchPolicyData
var patchFlag, patchWhenClosedFlag bool
if existingInstaller.FleetMaintainedAppID != nil {
existingPolicy, err = svc.ds.GetPatchPolicy(ctx, payload.TeamID, payload.TitleID)
if err != nil && !fleet.IsNotFound(err) {
return nil, ctxerr.Wrap(ctx, err, "getting patch policy")
}
patchFlag, patchWhenClosedFlag, err = planPatchPolicy(payload, existingInstaller, existingPolicy)
if err != nil {
return nil, err
}
}
// persist changes starting here, now that we've done all the validation/diffing we can
@@ -928,6 +938,30 @@ func (svc *Service) UpdateSoftwareInstaller(ctx context.Context, payload *fleet.
}
}
// Create, update, or delete the patch policy after the installer save
patchTeamID := ptr.ValOrZero(payload.TeamID)
switch {
case !patchFlag && existingPolicy != nil:
if _, err := svc.DeleteTeamPolicies(ctx, patchTeamID, []uint{existingPolicy.ID}); err != nil {
return nil, ctxerr.Wrap(ctx, err, "deleting patch policy")
}
case patchFlag && existingPolicy == nil:
patchType := fleet.PolicyTypePatch
if _, err := svc.NewTeamPolicy(ctx, patchTeamID, fleet.NewTeamPolicyPayload{
Type: &patchType,
PatchSoftwareTitleID: &payload.TitleID,
PatchWhenClosed: patchWhenClosedFlag,
// patch_when_closed requires continuous automations on; the create rejects it otherwise.
ContinuousAutomationsEnabled: patchWhenClosedFlag,
}); err != nil {
return nil, ctxerr.Wrap(ctx, err, "creating patch policy")
}
case patchFlag && existingPolicy != nil && patchWhenClosedFlag != existingPolicy.PatchWhenClosed:
if _, err := svc.ModifyTeamPolicy(ctx, patchTeamID, existingPolicy.ID, fleet.ModifyPolicyPayload{PatchWhenClosed: &patchWhenClosedFlag}); err != nil {
return nil, ctxerr.Wrap(ctx, err, "modifying patch policy")
}
}
// re-pull the edited installer to reflect side effects; return that specific
// package, not the title's first-added one. May be able to optimize this out later.
updatedInstaller, err := svc.ds.GetSoftwareInstallerMetadataByTeamTitleAndInstallerID(ctxdb.RequirePrimary(ctx, true), payload.TeamID, payload.TitleID, payload.InstallerID, true)
@@ -944,66 +978,43 @@ func (svc *Service) UpdateSoftwareInstaller(ctx context.Context, payload *fleet.
return updatedInstaller, nil
}
func (svc *Service) reconcilePatchPolicy(ctx context.Context, payload *fleet.UpdateSoftwareInstallerPayload, installer *fleet.SoftwareInstaller) error {
// Only the Fleet-maintained package has a managed pre-install query, so nothing here applies
// to any other package. Patch controls on a non-FMA package are rejected by the caller.
func planPatchPolicy(payload *fleet.UpdateSoftwareInstallerPayload, installer *fleet.SoftwareInstaller, existingPolicy *fleet.PatchPolicyData) (patchFlag bool, patchWhenClosedFlag bool, err error) {
// Only the Fleet-maintained package has a managed pre-install query; patch controls on any other
// package are rejected by the caller.
if installer.FleetMaintainedAppID == nil {
return nil
}
if payload.Patch == nil && payload.PatchWhenClosed == nil && payload.PreInstallQuery == nil {
return nil
return false, false, nil
}
existing, err := svc.ds.GetPatchPolicy(ctx, payload.TeamID, payload.TitleID)
if err != nil && !fleet.IsNotFound(err) {
return ctxerr.Wrap(ctx, err, "getting patch policy")
}
patchEnabled := existing != nil
if payload.Patch != nil {
patchEnabled = *payload.Patch
}
// patch_when_closed needs a patch policy: reject when patch is disabled, or when it's omitted
// and the title has no policy to enable it on.
if payload.PatchWhenClosed != nil && *payload.PatchWhenClosed && !patchEnabled {
return &fleet.BadRequestError{Message: `"patch_when_closed" requires "patch" to be enabled.`}
}
// Default patch_when_closed on for a new patch policy, otherwise keep the existing value.
patchWhenClosed := existing != nil && existing.PatchWhenClosed
switch {
case !patchEnabled:
patchWhenClosed = false
case payload.PatchWhenClosed != nil:
patchWhenClosed = *payload.PatchWhenClosed
case existing == nil:
patchWhenClosed = true
}
// The pre-install query is read-only while patch_when_closed is enabled.
if patchWhenClosed && payload.PreInstallQuery != nil {
return &fleet.BadRequestError{Message: `Couldn't edit. "pre_install_query" is managed by Fleet and can't be set directly while "patch_when_closed" is enabled.`}
}
teamID := ptr.ValOrZero(payload.TeamID)
switch {
case !patchEnabled:
if existing == nil {
return nil
// Resolve both optional flags into plain bools so the logic below never touches the pointers.
// An omitted patch keeps the current state.
if payload.Patch == nil {
if existingPolicy != nil {
patchFlag = true
}
_, err = svc.DeleteTeamPolicies(ctx, teamID, []uint{existing.ID})
case existing == nil:
patchType := fleet.PolicyTypePatch
_, err = svc.NewTeamPolicy(ctx, teamID, fleet.NewTeamPolicyPayload{
Type: &patchType,
PatchSoftwareTitleID: &payload.TitleID,
PatchWhenClosed: patchWhenClosed,
})
case patchWhenClosed != existing.PatchWhenClosed:
_, err = svc.ModifyTeamPolicy(ctx, teamID, existing.ID, fleet.ModifyPolicyPayload{PatchWhenClosed: &patchWhenClosed})
default:
return nil
} else {
patchFlag = *payload.Patch
}
return ctxerr.Wrap(ctx, err, "reconciling patch policy")
// An omitted patch_when_closed keeps the current value, or defaults on for a new policy.
if payload.PatchWhenClosed == nil {
if existingPolicy != nil {
patchWhenClosedFlag = existingPolicy.PatchWhenClosed
} else {
patchWhenClosedFlag = true
}
} else {
patchWhenClosedFlag = *payload.PatchWhenClosed
}
// patch_when_closed is only meaningful with patch enabled (in this request or already on the title).
if payload.PatchWhenClosed != nil && !patchFlag {
return false, false, &fleet.BadRequestError{Message: `If "patch_when_closed" is set, "patch" must be true.`}
}
// The pre-install query is read-only only while patch_when_closed will actually be in effect.
if patchFlag && patchWhenClosedFlag && payload.PreInstallQuery != nil {
return false, false, &fleet.BadRequestError{Message: `Couldn't edit. "pre_install_query" is managed by Fleet and can't be set directly while "patch_when_closed" is enabled.`}
}
return patchFlag, patchWhenClosedFlag, nil
}
func (svc *Service) validateEmbeddedSecretsOnScript(ctx context.Context, scriptName string, script *string,
+34 -71
View File
@@ -2652,115 +2652,78 @@ func TestNormalizeSetupExperiencePlatforms(t *testing.T) {
}
}
func TestReconcilePatchPolicy(t *testing.T) {
ctx := context.Background()
func TestPlanPatchPolicy(t *testing.T) {
titleID := uint(42)
teamID := uint(0)
fmaInstaller := &fleet.SoftwareInstaller{TitleID: &titleID, FleetMaintainedAppID: new(uint(7)), PreInstallQuery: "SELECT old;"}
nonFMAInstaller := &fleet.SoftwareInstaller{TitleID: &titleID}
// setup resolves GetPatchPolicy to existing (nil means the title has no patch policy).
setup := func(t *testing.T, existing *fleet.PatchPolicyData) (*Service, *svcmock.Service) {
ds := new(mock.Store)
ds.GetPatchPolicyFunc = func(ctx context.Context, gotTeamID *uint, gotTitleID uint) (*fleet.PatchPolicyData, error) {
if existing == nil {
return nil, &notFoundError{}
}
return existing, nil
}
return newTestServiceWithMock(t, ds)
}
payload := func(patch *bool, patchWhenClosed *bool) *fleet.UpdateSoftwareInstallerPayload {
return &fleet.UpdateSoftwareInstallerPayload{TitleID: titleID, TeamID: &teamID, Patch: patch, PatchWhenClosed: patchWhenClosed}
}
// patch_when_closed can't be enabled unless patch is enabled too, whether patch is omitted
// (with no existing policy) or explicitly disabled.
// patch_when_closed set without patch enabled is rejected, whether patch is omitted with no
// existing policy or explicitly disabled.
t.Run("rejects patch_when_closed without patch", func(t *testing.T) {
svc, _ := setup(t, nil)
require.ErrorContains(t, svc.reconcilePatchPolicy(ctx, payload(nil, new(true)), fmaInstaller), "requires")
require.ErrorContains(t, svc.reconcilePatchPolicy(ctx, payload(new(false), new(true)), fmaInstaller), "requires")
_, _, err := planPatchPolicy(payload(nil, new(true)), fmaInstaller, nil)
require.ErrorContains(t, err, `"patch" must be true`)
_, _, err = planPatchPolicy(payload(new(false), new(true)), fmaInstaller, nil)
require.ErrorContains(t, err, `"patch" must be true`)
})
// While patch_when_closed is on, the user pre-install query is managed and can't be edited.
t.Run("rejects pre-install edit while managed", func(t *testing.T) {
svc, _ := setup(t, &fleet.PatchPolicyData{ID: 9, PatchWhenClosed: true})
p := payload(nil, nil)
p.PreInstallQuery = new("SELECT changed;")
err := svc.reconcilePatchPolicy(ctx, p, fmaInstaller)
_, _, err := planPatchPolicy(p, fmaInstaller, &fleet.PatchPolicyData{ID: 9, PatchWhenClosed: true})
require.ErrorContains(t, err, "managed by Fleet")
})
// A pre-install edit on a non-FMA package is never managed.
// A pre-install edit on a non-FMA package is never managed; nothing to plan.
t.Run("allows pre-install edit on non-FMA package", func(t *testing.T) {
svc, _ := setup(t, &fleet.PatchPolicyData{ID: 9, PatchWhenClosed: true})
p := payload(nil, nil)
p.PreInstallQuery = new("SELECT changed;")
require.NoError(t, svc.reconcilePatchPolicy(ctx, p, nonFMAInstaller))
patchFlag, _, err := planPatchPolicy(p, nonFMAInstaller, &fleet.PatchPolicyData{ID: 9, PatchWhenClosed: true})
require.NoError(t, err)
assert.False(t, patchFlag)
})
// patch:true with no existing policy creates one with the requested patch_when_closed.
// patch:true with no existing policy plans a create with patch_when_closed on.
t.Run("creates when no policy exists", func(t *testing.T) {
svc, base := setup(t, nil)
base.NewTeamPolicyFunc = func(ctx context.Context, tID uint, p fleet.NewTeamPolicyPayload) (*fleet.Policy, error) {
require.NotNil(t, p.Type)
assert.Equal(t, fleet.PolicyTypePatch, *p.Type)
assert.True(t, p.PatchWhenClosed)
return &fleet.Policy{}, nil
}
require.NoError(t, svc.reconcilePatchPolicy(ctx, payload(new(true), new(true)), fmaInstaller))
assert.True(t, base.NewTeamPolicyFuncInvoked)
patchFlag, patchWhenClosedFlag, err := planPatchPolicy(payload(new(true), new(true)), fmaInstaller, nil)
require.NoError(t, err)
assert.True(t, patchFlag)
assert.True(t, patchWhenClosedFlag)
})
// patch:true with patch_when_closed omitted defaults a new policy to "only when closed".
t.Run("new policy defaults to patch_when_closed", func(t *testing.T) {
svc, base := setup(t, nil)
var got bool
base.NewTeamPolicyFunc = func(ctx context.Context, tID uint, p fleet.NewTeamPolicyPayload) (*fleet.Policy, error) {
got = p.PatchWhenClosed
return &fleet.Policy{}, nil
}
require.NoError(t, svc.reconcilePatchPolicy(ctx, payload(new(true), nil), fmaInstaller))
assert.True(t, got)
_, patchWhenClosedFlag, err := planPatchPolicy(payload(new(true), nil), fmaInstaller, nil)
require.NoError(t, err)
assert.True(t, patchWhenClosedFlag)
})
// patch:false deletes the existing patch policy.
t.Run("deletes when patch disabled", func(t *testing.T) {
svc, base := setup(t, &fleet.PatchPolicyData{ID: 9})
var deleted []uint
base.DeleteTeamPoliciesFunc = func(ctx context.Context, tID uint, ids []uint) ([]uint, error) {
deleted = ids
return ids, nil
}
require.NoError(t, svc.reconcilePatchPolicy(ctx, payload(new(false), nil), fmaInstaller))
assert.Equal(t, []uint{9}, deleted)
// patch:false disables the existing patch policy.
t.Run("disables when patch off", func(t *testing.T) {
patchFlag, _, err := planPatchPolicy(payload(new(false), nil), fmaInstaller, &fleet.PatchPolicyData{ID: 9})
require.NoError(t, err)
assert.False(t, patchFlag)
})
// Toggling patch_when_closed on an existing policy updates it rather than recreating it.
// Toggling patch_when_closed on an existing policy keeps patch on and flips the value.
t.Run("updates patch_when_closed on existing policy", func(t *testing.T) {
svc, base := setup(t, &fleet.PatchPolicyData{ID: 9, PatchWhenClosed: false})
var modified *bool
base.ModifyTeamPolicyFunc = func(ctx context.Context, tID uint, id uint, p fleet.ModifyPolicyPayload) (*fleet.Policy, error) {
assert.Equal(t, uint(9), id)
modified = p.PatchWhenClosed
return &fleet.Policy{}, nil
}
require.NoError(t, svc.reconcilePatchPolicy(ctx, payload(nil, new(true)), fmaInstaller))
assert.False(t, base.NewTeamPolicyFuncInvoked)
require.NotNil(t, modified)
assert.True(t, *modified)
patchFlag, patchWhenClosedFlag, err := planPatchPolicy(payload(nil, new(true)), fmaInstaller, &fleet.PatchPolicyData{ID: 9, PatchWhenClosed: false})
require.NoError(t, err)
assert.True(t, patchFlag)
assert.True(t, patchWhenClosedFlag)
})
// A pre-install edit is allowed and touches no policy when the title's patch policy has
// patch_when_closed off.
// A pre-install edit is allowed when the title's patch policy has patch_when_closed off.
t.Run("pre-install edit allowed when patch_when_closed is off", func(t *testing.T) {
svc, base := setup(t, &fleet.PatchPolicyData{ID: 9, PatchWhenClosed: false})
p := payload(nil, nil)
p.PreInstallQuery = new("SELECT changed;")
require.NoError(t, svc.reconcilePatchPolicy(ctx, p, fmaInstaller))
assert.False(t, base.NewTeamPolicyFuncInvoked)
assert.False(t, base.ModifyTeamPolicyFuncInvoked)
assert.False(t, base.DeleteTeamPoliciesFuncInvoked)
_, patchWhenClosedFlag, err := planPatchPolicy(p, fmaInstaller, &fleet.PatchPolicyData{ID: 9, PatchWhenClosed: false})
require.NoError(t, err)
assert.False(t, patchWhenClosedFlag)
})
}
+2 -2
View File
@@ -169,7 +169,7 @@ func defaultMacOSOpenQuery(bundleIdentifier string) string {
// - get processes by name - requires a lot of manual overrides
// - use the running_apps table - not reliable when run through orbit
// - use the "app" artifact in the homebrew cask - requires extra code to extract
openTemplate := "SELECT 1 WHERE NOT EXISTS (SELECT 1 FROM apps a JOIN processes p ON p.path LIKE concat(a.path, '/%%') WHERE a.bundle_identifier = '%s');"
openTemplate := "SELECT 1 WHERE NOT EXISTS (SELECT 1 FROM apps a JOIN processes p ON substr(p.path, 1, LENGTH(a.path) + 1) = concat(a.path, '/') WHERE a.bundle_identifier = '%s');"
return fmt.Sprintf(openTemplate, escapeSQLLiteral(bundleIdentifier))
}
@@ -235,7 +235,7 @@ var windowsOpenQueryOverrides = map[string]string{ //nolint:gosec // G101 false
"PyCharm Community Edition": "IN ('pycharm.exe','pycharm64.exe')",
"PyCharm Professional": "IN ('pycharm.exe','pycharm64.exe')",
"Rider": "IN ('rider.exe','rider64.exe')",
"RStudio": "IN ('rgui.exe','rsession.exe' 'rstudio.exe')",
"RStudio": "IN ('rgui.exe','rsession.exe','rstudio.exe')",
"RubyMine": "IN ('rubymine.exe','rubymine64.exe')",
"RustRover": "IN ('rustrover.exe','rustrover64.exe')",
"Spotify": "IN ('spotify.exe','spotifywebhelper.exe')",
+2 -2
View File
@@ -74,11 +74,11 @@ func TestGenerateOpenQuery(t *testing.T) {
// macOS resolves the app's install path from its bundle identifier and matches a process
// running from inside it.
got := patch_policy.GenerateOpenQuery("darwin", "org.mozilla.firefox", "")
require.Equal(t, "SELECT 1 WHERE NOT EXISTS (SELECT 1 FROM apps a JOIN processes p ON p.path LIKE concat(a.path, '/%') WHERE a.bundle_identifier = 'org.mozilla.firefox');", got)
require.Equal(t, "SELECT 1 WHERE NOT EXISTS (SELECT 1 FROM apps a JOIN processes p ON substr(p.path, 1, LENGTH(a.path) + 1) = concat(a.path, '/') WHERE a.bundle_identifier = 'org.mozilla.firefox');", got)
// Apostrophes in the bundle identifier are escaped so they can't break the literal.
got = patch_policy.GenerateOpenQuery("darwin", "com.oreilly.o'reilly", "")
require.Equal(t, "SELECT 1 WHERE NOT EXISTS (SELECT 1 FROM apps a JOIN processes p ON p.path LIKE concat(a.path, '/%') WHERE a.bundle_identifier = 'com.oreilly.o''reilly');", got)
require.Equal(t, "SELECT 1 WHERE NOT EXISTS (SELECT 1 FROM apps a JOIN processes p ON substr(p.path, 1, LENGTH(a.path) + 1) = concat(a.path, '/') WHERE a.bundle_identifier = 'com.oreilly.o''reilly');", got)
// Windows matches a process named "<title>.exe".
got = patch_policy.GenerateOpenQuery("windows", "", "Slack")
@@ -49,7 +49,7 @@ func TestUp_20260721173820(t *testing.T) {
`SELECT patch_when_closed FROM policies WHERE id = ?`, policy2))
assert.True(t, patchWhenClosed)
const managedQuery = "SELECT 1 WHERE NOT EXISTS (SELECT 1 FROM apps a JOIN processes p ON p.path LIKE concat(a.path, '/%') WHERE a.bundle_identifier = 'com.example.app');"
const managedQuery = "SELECT 1 WHERE NOT EXISTS (SELECT 1 FROM apps a JOIN processes p ON substr(p.path, 1, LENGTH(a.path) + 1) = concat(a.path, '/') WHERE a.bundle_identifier = 'com.example.app');"
title2 := execNoErrLastID(t, db,
`INSERT INTO software_titles (name, source, extension_for) VALUES ('App2', 'apps', '')`)
installer2 := execNoErrLastID(t, db, `
+7
View File
@@ -1782,6 +1782,13 @@ func (ds *Datastore) ApplyPolicySpecs(ctx context.Context, authorID uint, specs
if err != nil {
return ctxerr.Wrap(ctx, err, "getting patch policy installer")
}
// Defensive: this can only happen if this endpoint is being called not via gitops
// and batch software didn't get called before.
if spec.PatchWhenClosed && installer.PreInstallQuery != "" {
return ctxerr.Errorf(ctx, "policy %q: pre_install_query can't be set on Fleet-maintained app %q when patch_when_closed is true", spec.Name, spec.FleetMaintainedAppSlug)
}
generated, err := patch_policy.GenerateFromInstaller(patch_policy.PolicyData{
Name: spec.Name,
Description: spec.Description,
+59
View File
@@ -93,6 +93,7 @@ func TestPolicies(t *testing.T) {
{"PolicyModificationResetsAttemptNumber", testPolicyModificationResetsAttemptNumber},
{"TeamPatchPolicy", testTeamPatchPolicy},
{"ApplyPolicySpecsDynamicAndPatchSameFMA", testApplyPolicySpecsDynamicAndPatchSameFMA},
{"ApplyPolicySpecsPatchWhenClosedRejectsPreInstallQuery", testApplyPolicySpecsPatchWhenClosedRejectsPreInstallQuery},
{"ApplyPolicySpecsRenamePatchPolicyRegression43687", testApplyPolicySpecsRenamePatchPolicyRegression43687},
{"TeamPolicyAutomationFilter", testTeamPolicyAutomationFilter},
{"BatchedPolicyMembershipCleanup", testBatchedPolicyMembershipCleanup},
@@ -8333,6 +8334,64 @@ func testApplyPolicySpecsDynamicAndPatchSameFMA(t *testing.T, ds *Datastore) {
require.Equal(t, fmaTitleID, *patch.PatchSoftwareTitleID)
}
func testApplyPolicySpecsPatchWhenClosedRejectsPreInstallQuery(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-pwc-pre-install"})
require.NoError(t, err)
maintainedApp, err := ds.UpsertMaintainedApp(ctx, &fleet.MaintainedApp{
Name: "Maintained2",
Slug: "maintained2",
Platform: "darwin",
UniqueIdentifier: "fleet.maintained2",
})
require.NoError(t, err)
// The package carries a user-set pre-install query.
_, _, err = ds.MatchOrCreateSoftwareInstaller(ctx, &fleet.UploadSoftwareInstallerPayload{
InstallScript: "hello",
PreInstallQuery: "SELECT 1",
StorageID: "storage-pwc-pre-install",
Filename: "maintained2",
Title: "Maintained2",
Version: "1.0",
Source: "apps",
Platform: "darwin",
BundleIdentifier: "fleet.maintained2",
UserID: user1.ID,
TeamID: &team1.ID,
ValidatedLabels: &fleet.LabelIdentsWithScope{},
FleetMaintainedAppID: &maintainedApp.ID,
})
require.NoError(t, err)
spec := func(patchWhenClosed bool) []*fleet.PolicySpec {
return []*fleet.PolicySpec{{
Name: "patch-fma-when-closed",
Query: "SELECT 1;",
Team: team1.Name,
Type: fleet.PolicyTypePatch,
FleetMaintainedAppSlug: "maintained2",
PatchWhenClosed: patchWhenClosed,
}}
}
// patch_when_closed is rejected while the package has its own pre-install query.
err = ds.ApplyPolicySpecs(ctx, user1.ID, spec(true))
require.ErrorContains(t, err, "pre_install_query can't be set on Fleet-maintained app")
// The rejected batch wrote nothing.
var count int
ExecAdhocSQL(t, ds, func(q sqlx.ExtContext) error {
return sqlx.GetContext(ctx, q, &count, `SELECT COUNT(*) FROM policies WHERE name = ?`, "patch-fma-when-closed")
})
require.Zero(t, count)
// The same spec applies without patch_when_closed.
require.NoError(t, ds.ApplyPolicySpecs(ctx, user1.ID, spec(false)))
}
// testApplyPolicySpecsRenamePatchPolicyRegression43687 reproduces the customer
// scenario from issue #43687: GitOps renaming a patch policy that references
// an FMA (e.g. "Adobe Reader up to date" -> "Adobe Reader") used to 5xx with
@@ -1990,6 +1990,33 @@ func (ds *Datastore) ProcessInstallerUpdateSideEffects(ctx context.Context, inst
return ds.activateNextUpcomingActivityForBatchOfHosts(ctx, activateAffectedHostIDs)
}
func (ds *Datastore) ClearPreInstallQueryForTitle(ctx context.Context, teamID uint, titleID uint) error {
// An FMA title has one is_active=1 row, so team and title identify the managed installer.
var installer fleet.SoftwareInstaller
err := sqlx.GetContext(ctx, ds.writer(ctx), &installer, `
SELECT id, COALESCE(pre_install_query, '') AS pre_install_query
FROM software_installers
WHERE global_or_team_id = ?
AND title_id = ?
AND fleet_maintained_app_id IS NOT NULL
AND is_active = 1
LIMIT 1`, teamID, titleID)
switch {
case errors.Is(err, sql.ErrNoRows):
return nil
case err != nil:
return ctxerr.Wrap(ctx, err, "get title installer")
case installer.PreInstallQuery == "":
return nil
}
if _, err := ds.writer(ctx).ExecContext(ctx,
`UPDATE software_installers SET pre_install_query = '' WHERE id = ?`, installer.InstallerID); err != nil {
return ctxerr.Wrap(ctx, err, "clear pre-install query for title")
}
return ds.ProcessInstallerUpdateSideEffects(ctx, installer.InstallerID, true, false)
}
func (ds *Datastore) runInstallerUpdateSideEffectsInTransaction(ctx context.Context, tx sqlx.ExtContext, installerID uint, wasMetadataUpdated bool, wasPackageUpdated bool, isEdit bool) (affectedHostIDs []uint, err error) {
if wasMetadataUpdated || wasPackageUpdated { // cancel pending installs/uninstalls
// TODO make this less naive; this assumes that installs/uninstalls execute and report back immediately
@@ -7183,7 +7183,7 @@ func testGetSoftwareInstallDetailsPatchWhenClosed(t *testing.T, ds *Datastore) {
require.NoError(t, ds.AddHostsToTeam(ctx, fleet.NewAddHostsToTeamParams(&team.ID, []uint{host.ID})))
const userQuery = "SELECT 1 FROM user_configured_query;"
const managedQuery = "SELECT 1 WHERE NOT EXISTS (SELECT 1 FROM apps a JOIN processes p ON p.path LIKE concat(a.path, '/%') WHERE a.bundle_identifier = 'com.example.pwc');"
const managedQuery = "SELECT 1 WHERE NOT EXISTS (SELECT 1 FROM apps a JOIN processes p ON substr(p.path, 1, LENGTH(a.path) + 1) = concat(a.path, '/') WHERE a.bundle_identifier = 'com.example.pwc');"
// A Fleet-maintained-app-backed installer carries both the user pre-install query and the
// Fleet-managed app_open_query.
@@ -7226,6 +7226,10 @@ func testGetSoftwareInstallDetailsPatchWhenClosed(t *testing.T, ds *Datastore) {
})
require.NoError(t, err)
require.Equal(t, patchWhenClosed, p.PatchWhenClosed)
if patchWhenClosed {
// The service does this after writing the policy; call it here to exercise the same effect.
require.NoError(t, ds.ClearPreInstallQueryForTitle(ctx, team.ID, titleID))
}
return p
}
@@ -7246,12 +7250,13 @@ func testGetSoftwareInstallDetailsPatchWhenClosed(t *testing.T, ds *Datastore) {
require.NoError(t, err)
require.Equal(t, managedQuery, activatedDetails.PreInstallCondition)
// Same installer, but a manual (non-policy) install falls back to the user query.
// Same installer via a manual (non-policy) install: no pre-install condition, because enabling
// patch_when_closed cleared the installer's user query.
manualExec, err := ds.InsertSoftwareInstallRequest(ctx, host.ID, closedInstaller, fleet.HostSoftwareInstallOptions{})
require.NoError(t, err)
manualDetails, err := ds.GetSoftwareInstallDetails(ctx, manualExec)
require.NoError(t, err)
require.Equal(t, userQuery, manualDetails.PreInstallCondition)
require.Empty(t, manualDetails.PreInstallCondition)
// A patch policy without patch_when_closed keeps the user query on the policy path.
forceInstaller, forceTitle := newInstaller(t, "pwc-force")
@@ -7299,7 +7304,7 @@ func testSoftwareInstallerAppOpenQueryRoundTrip(t *testing.T, ds *Datastore) {
}
// The managed "is app open" query round-trips through create -> metadata read.
const managed = "SELECT 1 WHERE NOT EXISTS (SELECT 1 FROM apps a JOIN processes p ON p.path LIKE concat(a.path, '/%') WHERE a.bundle_identifier = 'com.example.app');"
const managed = "SELECT 1 WHERE NOT EXISTS (SELECT 1 FROM apps a JOIN processes p ON substr(p.path, 1, LENGTH(a.path) + 1) = concat(a.path, '/') WHERE a.bundle_identifier = 'com.example.app');"
titleID := newInstaller(t, "app-open-1", managed)
meta, err := ds.GetSoftwareInstallerMetadataByTeamAndTitleID(ctx, nil, titleID, false)
require.NoError(t, err)
+1 -1
View File
@@ -187,9 +187,9 @@ type TeamPolicyRequest struct {
LabelsExcludeAll []string `json:"labels_exclude_all" premium:"true"`
ConditionalAccessEnabled bool `json:"conditional_access_enabled"`
ContinuousAutomationsEnabled bool `json:"continuous_automations_enabled" premium:"true"`
PatchWhenClosed bool `json:"patch_when_closed" premium:"true"`
Type *string `json:"type"`
PatchSoftwareTitleID *uint `json:"patch_software_title_id"`
PatchWhenClosed bool `json:"patch_when_closed" premium:"true"`
}
type TeamPolicyResponse struct {
+4
View File
@@ -2931,6 +2931,10 @@ type Datastore interface {
// to how the virtual column works).
ProcessInstallerUpdateSideEffects(ctx context.Context, installerID uint, wasMetadataUpdated bool, wasPackageUpdated bool) error
// ClearPreInstallQueryForTitle blanks the pre-install query on a title's active Fleet-maintained
// installer and cancels its pending installs. No-op when the query is already empty.
ClearPreInstallQueryForTitle(ctx context.Context, teamID uint, titleID uint) error
// SaveInstallerUpdates persists new values to an existing installer. See comments in the payload struct
// for which fields must be set.
SaveInstallerUpdates(ctx context.Context, payload *UpdateSoftwareInstallerPayload) error
+12
View File
@@ -1656,6 +1656,8 @@ type UpdateInstallerUpgradeCodeFunc func(ctx context.Context, id uint, upgradeCo
type ProcessInstallerUpdateSideEffectsFunc func(ctx context.Context, installerID uint, wasMetadataUpdated bool, wasPackageUpdated bool) error
type ClearPreInstallQueryForTitleFunc func(ctx context.Context, teamID uint, titleID uint) error
type SaveInstallerUpdatesFunc func(ctx context.Context, payload *fleet.UpdateSoftwareInstallerPayload) error
type UpdateInstallerSelfServiceFlagFunc func(ctx context.Context, selfService bool, id uint) error
@@ -4687,6 +4689,9 @@ type DataStore struct {
ProcessInstallerUpdateSideEffectsFunc ProcessInstallerUpdateSideEffectsFunc
ProcessInstallerUpdateSideEffectsFuncInvoked bool
ClearPreInstallQueryForTitleFunc ClearPreInstallQueryForTitleFunc
ClearPreInstallQueryForTitleFuncInvoked bool
SaveInstallerUpdatesFunc SaveInstallerUpdatesFunc
SaveInstallerUpdatesFuncInvoked bool
@@ -11275,6 +11280,13 @@ func (s *DataStore) ProcessInstallerUpdateSideEffects(ctx context.Context, insta
return s.ProcessInstallerUpdateSideEffectsFunc(ctx, installerID, wasMetadataUpdated, wasPackageUpdated)
}
func (s *DataStore) ClearPreInstallQueryForTitle(ctx context.Context, teamID uint, titleID uint) error {
s.mu.Lock()
s.ClearPreInstallQueryForTitleFuncInvoked = true
s.mu.Unlock()
return s.ClearPreInstallQueryForTitleFunc(ctx, teamID, titleID)
}
func (s *DataStore) SaveInstallerUpdates(ctx context.Context, payload *fleet.UpdateSoftwareInstallerPayload) error {
s.mu.Lock()
s.SaveInstallerUpdatesFuncInvoked = true
+6 -1
View File
@@ -256,7 +256,7 @@ func (svc Service) removeGlobalPoliciesFromWebhookConfig(ctx context.Context, id
const (
errPolicyAllFleetsForConditionalAccess = "\"All fleets\" policy cannot have conditional_access_enabled set"
errPolicyAllFleetsForContinuousAutomations = "\"All fleets\" policy cannot have continuous_automations_enabled set"
errPatchWhenClosedRequiresContinuousAutomations = "\"continuous_automations_enabled\" cannot be disabled while \"patch_when_closed\" is enabled"
errPatchWhenClosedRequiresContinuousAutomations = "If \"patch_when_closed\" is true, \"continuous_automations_enabled\" can't be set to false."
)
func modifyGlobalPolicyEndpoint(ctx context.Context, request interface{}, svc fleet.Service) (fleet.Errorer, error) {
@@ -500,6 +500,11 @@ func (svc *Service) ApplyPolicySpecs(ctx context.Context, policies []*fleet.Poli
return fleet.ErrMissingLicense
}
// PatchWhenClosed is premium-only.
if policy.PatchWhenClosed && !license.IsPremium(ctx) {
return fleet.ErrMissingLicense
}
// Make sure any applied labels exist.
labels := slices.Concat(policy.LabelsIncludeAny, policy.LabelsIncludeAll, policy.LabelsExcludeAny, policy.LabelsExcludeAll)
if len(labels) > 0 {
+48
View File
@@ -555,6 +555,54 @@ func TestApplyPolicySpecsLabelScopeRequiresPremium(t *testing.T) {
require.False(t, ds.ApplyPolicySpecsFuncInvoked)
}
func TestApplyPolicySpecsPatchWhenClosedRequiresPremium(t *testing.T) {
newDS := func() *mock.Store {
ds := new(mock.Store)
ds.AppConfigFunc = func(ctx context.Context) (*fleet.AppConfig, error) {
return &fleet.AppConfig{}, nil
}
ds.TeamByNameFunc = func(ctx context.Context, name string) (*fleet.Team, error) {
return &fleet.Team{ID: 1, Name: name}, nil
}
ds.ApplyPolicySpecsFunc = func(ctx context.Context, authorID uint, specs []*fleet.PolicySpec) error {
return nil
}
return ds
}
// patch_when_closed requires a patch-type team policy.
patchSpec := func() *fleet.PolicySpec {
return &fleet.PolicySpec{
Name: "patch policy",
Team: "team1",
Type: fleet.PolicyTypePatch,
PatchWhenClosed: true,
}
}
testAdmin := fleet.User{ID: 1, GlobalRole: new(fleet.RoleAdmin)}
// A free-tier caller can't apply patch_when_closed, and we never reach the datastore.
t.Run("free tier rejected", func(t *testing.T) {
ds := newDS()
svc, ctx := newTestService(t, ds, nil, nil)
viewerCtx := viewer.NewContext(ctx, viewer.Viewer{User: &testAdmin})
err := svc.ApplyPolicySpecs(viewerCtx, []*fleet.PolicySpec{patchSpec()})
require.ErrorIs(t, err, fleet.ErrMissingLicense)
require.False(t, ds.ApplyPolicySpecsFuncInvoked)
})
// A premium caller applies it successfully.
t.Run("premium accepted", func(t *testing.T) {
ds := newDS()
svc, ctx := newTestService(t, ds, nil, nil, &TestServerOpts{License: &fleet.LicenseInfo{Tier: fleet.TierPremium}})
viewerCtx := viewer.NewContext(ctx, viewer.Viewer{User: &testAdmin})
err := svc.ApplyPolicySpecs(viewerCtx, []*fleet.PolicySpec{patchSpec()})
require.NoError(t, err)
require.True(t, ds.ApplyPolicySpecsFuncInvoked)
})
}
func TestApplyPolicySpecsDefaultType(t *testing.T) {
ds := new(mock.Store)
ds.AppConfigFunc = func(ctx context.Context) (*fleet.AppConfig, error) {
+6
View File
@@ -261,6 +261,12 @@ func (svc *Service) SoftwareTitleByID(ctx context.Context, id uint, teamID *uint
return nil, ctxerr.Wrap(ctx, err, "get patch policy")
}
pkg.PatchPolicy = patchPolicy
// While patch_when_closed is on, the pre-install query is Fleet's managed
// app open query, shown read-only.
if patchPolicy != nil && patchPolicy.PatchWhenClosed {
pkg.PreInstallQuery = pkg.AppOpenQuery
}
}
}
+18 -8
View File
@@ -42,9 +42,9 @@ func teamPolicyEndpoint(ctx context.Context, request interface{}, svc fleet.Serv
LabelsExcludeAll: req.LabelsExcludeAll,
ConditionalAccessEnabled: req.ConditionalAccessEnabled,
ContinuousAutomationsEnabled: req.ContinuousAutomationsEnabled,
PatchWhenClosed: req.PatchWhenClosed,
Type: req.Type,
PatchSoftwareTitleID: req.PatchSoftwareTitleID,
PatchWhenClosed: req.PatchWhenClosed,
})
if err != nil {
return fleet.TeamPolicyResponse{Err: err}, nil
@@ -104,6 +104,12 @@ func (svc Service) NewTeamPolicy(ctx context.Context, teamID uint, tp fleet.NewT
return nil, ctxerr.Wrap(ctx, err, "populate automations")
}
if policy.Type == fleet.PolicyTypePatch && policy.PatchWhenClosed && policy.PatchSoftwareTitleID != nil {
if err := svc.ds.ClearPreInstallQueryForTitle(ctx, teamID, *policy.PatchSoftwareTitleID); err != nil {
return nil, ctxerr.Wrap(ctx, err, "clear pre-install query for title")
}
}
if teamID == 0 {
noTeamID := int64(0)
if err := svc.NewActivity(
@@ -304,9 +310,8 @@ func (svc *Service) newTeamPolicyPayloadToPolicyPayload(ctx context.Context, tea
}
// Continuous automations must be enabled so the patch policy keeps retrying until the app is closed.
continuousAutomationsEnabled := p.ContinuousAutomationsEnabled
if p.PatchWhenClosed {
continuousAutomationsEnabled = true
if p.PatchWhenClosed && !p.ContinuousAutomationsEnabled {
return fleet.PolicyPayload{}, &fleet.BadRequestError{Message: errPatchWhenClosedRequiresContinuousAutomations}
}
return fleet.PolicyPayload{
@@ -326,7 +331,7 @@ func (svc *Service) newTeamPolicyPayloadToPolicyPayload(ctx context.Context, tea
LabelsExcludeAny: p.LabelsExcludeAny,
LabelsExcludeAll: p.LabelsExcludeAll,
ConditionalAccessEnabled: p.ConditionalAccessEnabled,
ContinuousAutomationsEnabled: continuousAutomationsEnabled,
ContinuousAutomationsEnabled: p.ContinuousAutomationsEnabled,
PatchWhenClosed: p.PatchWhenClosed,
Type: policyType,
PatchSoftwareTitleID: p.PatchSoftwareTitleID,
@@ -713,10 +718,9 @@ func (svc *Service) modifyPolicy(ctx context.Context, teamID *uint, id uint, p f
if p.PatchWhenClosed != nil {
patchWhenClosed = *p.PatchWhenClosed
}
// patch_when_closed needs continuous automations: reject an explicit false, otherwise force it on.
if patchWhenClosed && p.ContinuousAutomationsEnabled != nil && !*p.ContinuousAutomationsEnabled {
return nil, ctxerr.Wrap(ctx, &fleet.BadRequestError{
Message: fmt.Sprintf("policy payload verification: %s", errPatchWhenClosedRequiresContinuousAutomations),
})
return nil, ctxerr.Wrap(ctx, &fleet.BadRequestError{Message: errPatchWhenClosedRequiresContinuousAutomations})
}
if patchWhenClosed {
policy.ContinuousAutomationsEnabled = true
@@ -797,6 +801,12 @@ func (svc *Service) modifyPolicy(ctx context.Context, teamID *uint, id uint, p f
return nil, ctxerr.Wrap(ctx, err, "populate automations")
}
if policy.Type == fleet.PolicyTypePatch && policy.PatchWhenClosed && policy.PatchSoftwareTitleID != nil {
if err := svc.ds.ClearPreInstallQueryForTitle(ctx, ptr.ValOrZero(teamID), *policy.PatchSoftwareTitleID); err != nil {
return nil, ctxerr.Wrap(ctx, err, "clear pre-install query for title")
}
}
if teamID == nil {
globalTeamID := int64(-1)
if err := svc.NewActivity(
+40 -14
View File
@@ -237,17 +237,22 @@ func TestTeamPolicyPatchWhenClosed(t *testing.T) {
ds.GetSoftwareInstallerMetadataByTeamAndTitleIDFunc = func(ctx context.Context, tID *uint, titleID uint, withScriptContents bool) (*fleet.SoftwareInstaller, error) {
return &fleet.SoftwareInstaller{TitleID: new(patchSoftwareTitleID), SoftwareTitle: "App", DisplayName: "App"}, nil
}
ds.ClearPreInstallQueryForTitleFunc = func(ctx context.Context, teamID uint, titleID uint) error {
return nil
}
return ds
}
// Creating a patch-when-closed policy forces continuous automations on, even when the
// request left the field at its default false.
t.Run("create auto-sets continuous automations", func(t *testing.T) {
// Creating a patch-when-closed policy with continuous automations on succeeds and clears the
// title's managed pre-install query.
t.Run("create patch-when-closed policy", func(t *testing.T) {
ds := setupDS()
var captured fleet.PolicyPayload
ds.NewTeamPolicyFunc = func(ctx context.Context, tID uint, authorID *uint, args fleet.PolicyPayload) (*fleet.Policy, error) {
captured = args
return freshPatchPolicy(), nil
created := freshPatchPolicy()
created.PatchWhenClosed = true
return created, nil
}
opts := &TestServerOpts{}
svc, baseCtx := newTestService(t, ds, nil, nil, opts)
@@ -256,13 +261,30 @@ func TestTeamPolicyPatchWhenClosed(t *testing.T) {
}
_, err := svc.NewTeamPolicy(adminCtx(baseCtx), teamID, fleet.NewTeamPolicyPayload{
Type: &patchType,
PatchSoftwareTitleID: new(patchSoftwareTitleID),
PatchWhenClosed: true,
Type: &patchType,
PatchSoftwareTitleID: new(patchSoftwareTitleID),
PatchWhenClosed: true,
ContinuousAutomationsEnabled: true,
})
require.NoError(t, err)
assert.True(t, captured.PatchWhenClosed)
assert.True(t, captured.ContinuousAutomationsEnabled, "patch_when_closed should force continuous automations on")
assert.True(t, captured.ContinuousAutomationsEnabled)
// enabling patch_when_closed cancels the title's pending installs so they re-evaluate
assert.True(t, ds.ClearPreInstallQueryForTitleFuncInvoked)
})
// continuous_automations_enabled=false with patch_when_closed=true is rejected on create too.
t.Run("create rejects disabling continuous automations", func(t *testing.T) {
ds := setupDS()
svc, baseCtx := newTestService(t, ds, nil, nil)
_, err := svc.NewTeamPolicy(adminCtx(baseCtx), teamID, fleet.NewTeamPolicyPayload{
Type: &patchType,
PatchSoftwareTitleID: new(patchSoftwareTitleID),
PatchWhenClosed: true,
ContinuousAutomationsEnabled: false,
})
require.Error(t, err)
require.ErrorContains(t, err, "continuous_automations_enabled")
})
// patch_when_closed only applies to patch policies.
@@ -270,16 +292,18 @@ func TestTeamPolicyPatchWhenClosed(t *testing.T) {
ds := setupDS()
svc, baseCtx := newTestService(t, ds, nil, nil)
_, err := svc.NewTeamPolicy(adminCtx(baseCtx), teamID, fleet.NewTeamPolicyPayload{
Name: "dynamic policy",
Query: "SELECT 1;",
PatchWhenClosed: true,
Name: "dynamic policy",
Query: "SELECT 1;",
// Continuous automations must be on, otherwise that check rejects the payload first.
PatchWhenClosed: true,
ContinuousAutomationsEnabled: true,
})
require.Error(t, err)
require.ErrorContains(t, err, "patch_when_closed")
require.ErrorContains(t, err, "only supported for patch policies")
})
// Continuous automations can't be turned off in the same request that keeps
// patch_when_closed on.
// An explicit continuous_automations_enabled=false alongside patch_when_closed=true is rejected;
// omitting it (see next case) still auto-sets it to true.
t.Run("modify rejects disabling continuous automations", func(t *testing.T) {
ds := setupDS()
svc, baseCtx := newTestService(t, ds, nil, nil)
@@ -312,6 +336,8 @@ func TestTeamPolicyPatchWhenClosed(t *testing.T) {
require.NotNil(t, saved)
assert.True(t, saved.PatchWhenClosed)
assert.True(t, saved.ContinuousAutomationsEnabled)
// enabling patch_when_closed cancels the title's pending installs so they re-evaluate
assert.True(t, ds.ClearPreInstallQueryForTitleFuncInvoked)
})
}