[EDR Workflows] Set Agent Tamper Protection to false on policy unassignment

[szwarckonrad]

This PR fixes an issue where Agent Tamper Protection remained enabled even after a Defend policy was unassigned from the agent policy.

There is no issue with removing the integration; it still causes agent tamper protections to be disabled.

Screen.Recording.2024-09-16.at.15.51.26.mov

[elasticmachine]

Pinging @elastic/fleet (Team:Fleet)

[elasticmachine]

Pinging @elastic/security-defend-workflows (Team:Defend Workflows)

[nchaulet]

code LGTM 🚀

[gergoabraham]

Sets instead of arrays could be built here, so the checks in lines 1231 and 1232 will have constant time complexity

[szwarckonrad]

const [endpointPackagePolicyUpdatesIds, endpointOldPackagePoliciesIds] = [
  packagePolicyUpdates,
  oldPackagePolicies,
].map((packagePolicies) =>
  new Set(
    packagePolicies
      .filter((p) => p.package?.name === 'endpoint')
      .map((p) => p.policy_ids)
      .flat()
  )
);

Like this, right?

[gergoabraham]

like that : )

[ashokaditya]

May I also add that instead of iterating twice using filter and map you could use reduce to iterate only once. 😅

[gergoabraham]

works well when unassigning a Defend integration!

what do you think about initializing the is_protected field when a Defend integration is assigned to an agent policy? it should be false, but we could make sure

[szwarckonrad]

@gergoabraham

Hm, so you propose to add another check that will make sure (both on create and update) that if id is to be found in the "new" array and not in the "old" one, we should explicitly set is_protected to false as well? Can we think of any scenario where is_protected could not be false?

[gergoabraham]

@gergoabraham

Hm, so you propose to add another check that will make sure (both on create and update) that if id is to be found in the "new" array and not in the "old" one, we should explicitly set is_protected to false as well? Can we think of any scenario where is_protected could not be false?

@szwarckonrad, i don't actually know any scenario where it could be a problem, i'm just thinking on some weird possible SDHs where users added Defend integration and all of a sudden cannot uninstall agents, because some months prior to that somehow the is_protected field was changed to true. it most probably cannot happen (e.g. api call won't allow it, SO write in dev console requires tons of privileges), still, it just seems clearer to initialize the variable at the moment when we start using it.

so yeah, probably we don't need this, so if it would be a big change, we can definitely let it go, but if it's only a small one, i'd say let's go for it

what are your thoughts?

[gergoabraham]

awesome, thanks for the changes! 🚀

[nchaulet]

🚀

[kibana-ci]

💛 Build succeeded, but was flaky

• Buildkite Build
• Commit: 031eef3

Failed CI Steps

Metrics [docs]

✅ unchanged

History

• 💛 Build #235888 was flaky 7f58756
• 💚 Build #235454 succeeded 93f241f
• 💚 Build #234563 succeeded 67cd49c
• 💔 Build #234448 failed 51f4c63

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @szwarckonrad

[kibanamachine]

💚 All backports created successfully

Status	Branch	Result
✅	8.x

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation