search

fleetdm / fleet

Open-source platform for IT, security, and infrastructure teams. (Linux, macOS, Chrome, Windows, cloud, data center)
https://fleetdm.com
Other
3.02k stars 419 forks source link

No author displayed for scripts executed by policy automation #22692

Open iansltx opened 2 days ago

iansltx commented 2 days ago

Fleet version: 4.58.0

💥  Actual behavior

"Script ran" activities have no author in the host view either prior to or after execution when the scripts were run due to a policy automation (#17129). In the UI this shows as "ran script xyz.sh" rather than e.g. "Y Policy ran script xyz.sh."

For #19151 we add install authorship based on whether the package is self-serviceable, as either no one (self-service) or the uploader of the installer (no self-service). We don't have script authorship info stored currently (but we didn't before 4.57.0 for packages), though we do have authorship info and an edit audit trail for policies.

🧑‍💻  Steps to reproduce

  1. Associate a script to a policy
  2. Fail the policy on a host
  3. See an upcoming script run activity with a blank author
  4. Wait for the script to run on the host
  5. See the past script script runa ctivity with a blank author

🕯️ More info

It feels like the most useful thing to do here would be to store the polciy ID that triggered the script execution on the execution, then hydrate that into the activity along with the policy name at the time when the script runs. This gives more fine-grained detail on why the script ran than just populating who created the policy (or who uploaded the script, if we added those columns).

Note that software install automations don't take this approach. I think there's an argument to be made that they should, as it's pretty likely that the number of policies will be significantly higher than the number of people creating those policies (or uploading those installers), and the additional context of showing which policy failure triggered an action would be useful to IT admins and the like viewing host history. With that said, reworking software install automation activities would be out of scope for this particular bug, though easy enough to create a follow-on issue with a similar change.

🛠️ To fix

22690 is in draft to add policy ID to script executions, and policy ID + name to script run activities, with author name populated as policy name. This fix also includes a COALESCE on upcoming events, so the policy name will show there as long as the policy doesn't get deleted between script queue and execution.

iansltx commented 2 days ago

Based on discussions with @marko-lisica and @noahtalerman, given that #21681 is incoming, we'll:

  1. Mostly keep the revised behavior I've built
  2. Use "Fleet" as the author name, rather than policy name
  3. Match software install behavior to this pattern (will be another issue)
noahtalerman commented 2 days ago

Thanks @iansltx!

Match software install behavior to this pattern (will be another issue)

Do we need to file a separate bug for this one? Happy to help.

If I'm understanding correctly, when we ship the fix for this bug, script runs (from policy automations) will have "Fleet" as author.

Software installs will still have missing/ghost authors.

iansltx commented 2 days ago

Correct that this is specifically for script runs, not software installs.

Software installs maintain existing behavior, which shows the most recent uploader of the installer or ghost depending on self-service flags, and has a decent chunk of edge cases, some of which aren't yet accounted for.

Re: the other bug, I believe I have an existing bug outstanding whose fix can be rescoped to this.

noahtalerman commented 2 days ago

Re: the other bug, I believe I have an existing bug outstanding whose fix can be rescoped to this.

@iansltx you're right. I think this is the bug: https://github.com/fleetdm/fleet/issues/22087

I updated the issue description of that^ bug and added the same Fleet author fix in the "To fix" section.

UPDATE: I messed up. See later comment here (noahtalerman)

iansltx commented 2 days ago

@noahtalerman So, #22424 is the one I was going to pin this to. Behavior on edit should maintain the author.

noahtalerman commented 2 days ago

@iansltx ah shoot. I think I messed up. I reverted the description in #22087.

Software installs maintain existing behavior, which shows the most recent uploader of the installer or ghost depending on self-service flags, and has a decent chunk of edge cases, some of which aren't yet accounted for.

Filed a fresh bug for this here: https://github.com/fleetdm/fleet/issues/22705

iansltx commented 23 hours ago

@xpkoala test plan for this is basically:

  1. Add script
  2. Set up policy
  3. Attach script to policy
  4. Add a host failing the policy
  5. Policy failure should queue up the script, under Upcoming, with Fleet as the actor
  6. Once script run is completed, actor should be Fleet in global/host activity log

As regression tests, the following should still be true: