macOS: fix EndpointSecurity FIM mute inversion for file paths

[sharvilshah]

Slack conversation: https://osquery.slack.com/archives/C08VA3XQU/p1697483297449949

tl;dr: the bug was that we were inadvertently calling the ES mute inversion APIs on each config refresh, causing us to actually lose the mute inversion for file paths and cause a surge of events.

The fix is to check the ES mute inversion state, and call those APIs only if we are not currently "mute inverted" ( in ES parlance ES_MUTE_NOT_INVERTED). This essentially gates the existing code with:

if (es_muting_inverted(es_file_client_, ES_MUTE_INVERSION_TYPE_TARGET_PATH) == ES_MUTE_NOT_INVERTED) {
...
}

[sharvilshah]

closing and reopening to trigger latest CI

[Smjert]

A part from the inversion logic, something else I noticed is that the flags we are using here, are only evaluated once, when the publisher starts (which basically means when osquery starts).

I would define those as CLI_FLAG instead of FLAG to reflect that.

EDIT: Nevermind, I noticed things late and what I suggested is a breaking change. Nonetheless I think that we should remember to mark flags whose effect cannot (or must not) change at runtime as CLI_FLAG to reflect that to users.

[Smjert]

@directionless sorry if I caused confusion, but when I wrote "this is a breaking change" I meant my suggestion, not the PR here.

[sharvilshah]

@directionless I don't think this is a breaking/API change, so I am removing that label. Please feel free to add it if you think it requires one.

[directionless]

I'm not really following this, and I haven't absorbed this PR (pr the feature it's adjusting) but the conversation here makes me think you're dealing with cases around config changes and how it impacts the FIM subscriptions and mutes. Yes? If so, that leads me to questions... I won't be offended if I'm offbase, or y'all have covered/dismissed/whatever this direction.

1. Is the underlying issue that unrelated config changes causes spurious reprocessing of the FIM directives, which then cause things to get messed up?
2. Or that reprocessing the FIM directives is hard because of how it's layered together?
3. both?

I think one way to address (1) would be to keep some hash of the FIM config and early return if it's unchanged. This wouldn't address (2) but it might be part of it.

If I think about that kind of complex processing, I think it's very hard to do stateless. I'd end up tracking state, so that subsequent config parses could correlate with the state of what's already configured.

[Smjert]

I'm not really following this, and I haven't absorbed this PR (pr the feature it's adjusting) but the conversation here makes me think you're dealing with cases around config changes and how it impacts the FIM subscriptions and mutes. Yes? If so, that leads me to questions... I won't be offended if I'm offbase, or y'all have covered/dismissed/whatever this direction.

1. Is the underlying issue that unrelated config changes causes spurious reprocessing of the FIM directives, which then cause things to get messed up?

2. Or that reprocessing the FIM directives is hard because of how it's layered together?

3. both?

I think one way to address (1) would be to keep some hash of the FIM config and early return if it's unchanged. This wouldn't address (2) but it might be part of it.

If I think about that kind of complex processing, I think it's very hard to do stateless. I'd end up tracking state, so that subsequent config parses could correlate with the state of what's already configured.

At every config refresh osquery calls the configure function of a publisher. Publishers for FIM normally parse the file_paths config and re-apply it.
This is what this feature was originally attempting to do; due to issues with how the ES API works and how they were used, it was incorrectly inverting the filtering, basically filtering nothing.

The PR here fixes that behavior making so that the inversion happens only once (the inversion is necessary because you don't have 2 APIs, one for including a path and another for excluding it, so you need to use es_mute_path to do both, but for one of the two behaviors you need to call another API to invert the es_mute_path behavior).

This was working ok, but what I noticed was that we were not reacting to file_paths changes anymore. Then there has been some confusion on my part on the intentions, and now we are back to the original state of this PR, because Sharvil would like to address the "reacting to file_paths changes" later.

So no, the issue was not detecting when things change, but what and how to apply it correctly.

EDIT: Also to be extra clear, this is not a regression on some behavior that was present in previous releases. The support for file_paths has been added in this release; simply it will not be full featured.

[Smjert]

As discussed I think this is acceptable, improvements (reacting to file_paths changes) will come in another PR.

I'm approving but lets leave @directionless time to look at it.

[directionless]

If you two are happy, I don't feel like I need to stick my nose in. I'll merge