Hey folks, <@U0F2G55TM> and I identified a potenti...
# core
c
Hey folks, @sharvil and I identified a potential issue with 5.10.1. The TL;DR is that when an osquery API manager triggers a config refresh, it can cause the file monitoring targets list to get kinda messed up and start monitoring all filepaths which causes a DB/logging blowup. What are the options for: 1. Waiting on the release until this is fixed or 2. Quickly pushing out a new release with a fix (but not waiting an entire release cycle to do so)?
s
So I wouldn't wait, and open a PR first (if there's a fix). As for what to do/for when it gets in, given that we are in pre-release phase, I'll let that to the TSC. But to me it all depends on how big the fix is, and how much there's to wait, if there's a workaround, and if reverting the change is maybe just faster. The release is already a bit late. I would consider maybe reverting since this is the version that brings it in.
c
Makes sense, thanks! @sharvil thoughts?
s
I too wouldn’t want to hold up 5.10.1 release. On reverting the change: happy to take your lead, but I think that new feature is insulated by the flag in the table. While I am working on a fix, will work to get a faster 5.10.2 out
s
Yeah but I feel that shipping with a new feature (coming from this version) that's known not to work properly and with no workaround beyond not using it, it's bound to cause confusion I think. I mean I understand that it works if you don't do configs from a backend, but how common is that?
c
Yeah I’d be ok holding the release back until this is fixed (personally) When this breaks it’s kind of catastrophic in terms of the DB filling with events and sending them to our SIEM
s
makes sense, thanks Stefano.
can’t seem to revert the commit in the UI:
Sorry, this pull request couldn't be reverted automatically. It may have already been reverted, or the content may have changed since it was merged.
Will create a branch and try it that way
s
So I understand… Y’all think that the table (which is introduced in 5.10) is too broken. So it’s better to revert it, and cut a 5.10.2 release. And then implement a fix for 5.11?
c
I don't know how much work is involved in patching the issue, but I would really really really not like to wait an entire new release cycle to have a working version of this in osquery. Is there a world where if a PR with the patch was out soon, we'd just cut a 5.10.2 release that included it?
s
5.10.1 is near stable. I think Either this is stable enough and goes out as is, or we yank it for 5.10.2, and it lands in 5.11
c
lol it's probably "stable enough" in that no one will even know this feature got added unless they're closely monitoring the release notes. Whatever shortens the amount of time for "version of osquery that contains the patch is released" is my preference
s
Unless folks have a fix at the ready (days?) I don’t see this landing in 5.10
j
btw, which is the feature in question, @clong?
s
I’m not up on the technical side, I’m thinking about it from a release timing perspective.
c
file_events supporting the
open
event_type on MacOS You have to enable a bunch of flags to enable it:
Copy code
--enable_file_events
--disable_endpointsecurity=false
--disable_endpointsecurity_fim=false
--es_fim_enable_open_events=true
👍 1
s
It is gated behind a bunch of flags, and disabled by default, so as Chris says, it’s unlikely to affect folks
s
Right. I’d feel more strongly if it had broader impact.
s
Unless folks have a fix at the ready (days?) I don’t see this landing in 5.10
I am working on a fix, cautiously optimistic that it can done in a few days
But I also know that we were a bit behind cutting a 5.10
s
I think two interlocked questions: 1. Does this merit a 5.10.2? We’re unlikely to declare 5.10.1 stable before office hours, so maybe we can just defer? 2. Does this merit a revert, or roll forward. I’m happy to ignore this a couple days. and make a more informed decision after you tinker at fixing it
s
Just to clarify though, since I may have misunderstood earlier, is the interest here having the
open
event or the filtering, or both. Because the issue is in the filtering (as far as I understood?), but nothing prevents (I think) having the
open
event added? Also again, is the
open
event bugged or the filtering?
s
Yes, so we usually just roll forward. If we are not declaring it stable in the next couple of days, happy with the defer
c
@Stefano Bonicatti well, having the open event without the filtering means you're monitoring every open request across the whole file system which presents.....problems
s
The interest is in having both open events and filtering, the bug is in the filtering side of things
👍 1
s
Just to clarify and be transparent, I have no technical issue with either solution, my only technical suggestion was to no let 5.10.1 ship with that issue (in one way or another). At the same time though, I have customers who would like to have the release sooner rather than later (I mentioned the next office hours as a possibility), so this part is more of a favor, and you're free to ignore me.
👍 1
c
Do we want to set a deadline here or something? As in, if we cant get a working patch by $deadline, the release goes out?
s
I have a fix up! https://github.com/osquery/osquery/pull/8166 It’s a small change
Any chance on getting a quick review on that PR? The CI is passing, thank you!
c
cc @Stefano Bonicatti @seph
s
❤️ 2