[System] Fix AccessList & AccessMask processing in security data_stream#2156
[System] Fix AccessList & AccessMask processing in security data_stream#2156leehinman merged 1 commit intoelastic:masterfrom
Conversation
|
Pinging @elastic/security-external-integrations (Team:Security-External Integrations) |
💚 Build Succeeded
Expand to view the summary
Build stats
🤖 GitHub commentsTo re-run your PR in the CI, just comment with:
|
Could you elaborate on what problem this solves or error messages this corrects. |
There was a problem hiding this comment.
What's the source of these values? I recommend adding a comment in case we need to refresh the list in the future.
There was a problem hiding this comment.
Ideally it would store the key as a long in order to avoid having to repeatedly parse each string to a number. But IIRC there is a bug in that hex values are always strings anyways (elastic/elasticsearch#66555). So you would have to convert the values to decimal to make that work. Not sure it's worth the loss in clarity.
There was a problem hiding this comment.
We have similar logic in a few other places. I think I'll leave it as is for now, and we can change them all to Longs. We should be able to keep clarity with some well placed comments.
- According to MS documentation and AccessList contains a space separated list of access masks and AccessMask contains an integer. - Old code treated AccessMask as if it was a space separated list of access masks, this was causing script errors. - Fix code to treat AccessList as space separated list of access masks - Add new code to parse AccessMask correctly
b0027cb to
edc708c
Compare
What does this PR do?
Bug fix for processing AccessList and AccessMask in System security data_stream.
Checklist
changelog.ymlfile.- [ ] If I'm introducing a new feature, I have modified the Kibana version constraint in my package'smanifest.ymlfile to point to the latest Elastic stack release (e.g.^7.13.0).How to test this PR locally
elastic-package test