[Bug] Fix test_os_and_platform_in_query test and rules #3695
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
brokensound77 added bug brokensound77 requested review from Mikaayenson and eric-forte-elastic as code owners 
Mikaayenson reviewed May 18, 2024
Comment on lines +54 to +57
| if rule.path.parent.name == "windows": | ||
| if not any(field.startswith("winlog.") for field in fields): | ||
| self.assertIn("host.os.type", fields, err_msg) | ||
| else: |
There was a problem hiding this comment.
nit:
By the time we get here, we've already checked "host.os.type" not in fields:, so we probably dont need to assertIn again. Something like self.assertTrue(any(field.startswith("winlog.") for field in fields), "some more appropriate message about winlog and host.os.type" might be better.
Then in the else, self.fail(err_msg)
There was a problem hiding this comment.
I think you're reading it wrong. If winlog fields are present, the check is abandoned.
I initially had it collapsed more but it was much more readable this way.
There was a problem hiding this comment.
I mean compare line 52 & 56 for example or 52 & 58.
eric-forte-elastic approved these changes May 20, 2024
w0rk3r approved these changes May 20, 2024
protectionsmachine pushed a commit that referenced this pull request May 20, 2024
Co-authored-by: brokensound77 <brokensound77@users.noreply.github.com> Removed changes from: - rules/macos/credential_access_promt_for_pwd_via_osascript.toml - rules/macos/credential_access_suspicious_web_browser_sensitive_file_access.toml - rules/macos/initial_access_suspicious_mac_ms_office_child_process.toml (selectively cherry picked from commit ce21ace)
protectionsmachine pushed a commit that referenced this pull request May 20, 2024
Co-authored-by: brokensound77 <brokensound77@users.noreply.github.com> Removed changes from: - rules/macos/credential_access_promt_for_pwd_via_osascript.toml - rules/macos/credential_access_suspicious_web_browser_sensitive_file_access.toml - rules/macos/initial_access_suspicious_mac_ms_office_child_process.toml (selectively cherry picked from commit ce21ace)
protectionsmachine pushed a commit that referenced this pull request May 20, 2024
Co-authored-by: brokensound77 <brokensound77@users.noreply.github.com> Removed changes from: - rules/macos/credential_access_promt_for_pwd_via_osascript.toml - rules/macos/credential_access_suspicious_web_browser_sensitive_file_access.toml - rules/macos/initial_access_suspicious_mac_ms_office_child_process.toml (selectively cherry picked from commit ce21ace)
protectionsmachine pushed a commit that referenced this pull request May 20, 2024
Co-authored-by: brokensound77 <brokensound77@users.noreply.github.com> Removed changes from: - rules/macos/credential_access_promt_for_pwd_via_osascript.toml - rules/macos/credential_access_suspicious_web_browser_sensitive_file_access.toml - rules/macos/initial_access_suspicious_mac_ms_office_child_process.toml (selectively cherry picked from commit ce21ace)
protectionsmachine pushed a commit that referenced this pull request May 20, 2024
Co-authored-by: brokensound77 <brokensound77@users.noreply.github.com> Removed changes from: - rules/macos/credential_access_suspicious_web_browser_sensitive_file_access.toml - rules/macos/initial_access_suspicious_mac_ms_office_child_process.toml (selectively cherry picked from commit ce21ace)
protectionsmachine pushed a commit that referenced this pull request May 20, 2024
Co-authored-by: brokensound77 <brokensound77@users.noreply.github.com> Removed changes from: - rules/macos/credential_access_suspicious_web_browser_sensitive_file_access.toml - rules/macos/initial_access_suspicious_mac_ms_office_child_process.toml (selectively cherry picked from commit ce21ace)
protectionsmachine pushed a commit that referenced this pull request May 20, 2024
Co-authored-by: brokensound77 <brokensound77@users.noreply.github.com> Removed changes from: - rules/macos/credential_access_suspicious_web_browser_sensitive_file_access.toml - rules/macos/initial_access_suspicious_mac_ms_office_child_process.toml (selectively cherry picked from commit ce21ace)
protectionsmachine pushed a commit that referenced this pull request May 20, 2024
Co-authored-by: brokensound77 <brokensound77@users.noreply.github.com> Removed changes from: - rules/macos/credential_access_suspicious_web_browser_sensitive_file_access.toml - rules/macos/initial_access_suspicious_mac_ms_office_child_process.toml (selectively cherry picked from commit ce21ace)
protectionsmachine pushed a commit that referenced this pull request May 20, 2024
Co-authored-by: brokensound77 <brokensound77@users.noreply.github.com> (cherry picked from commit ce21ace)
protectionsmachine pushed a commit that referenced this pull request May 20, 2024
Co-authored-by: brokensound77 <brokensound77@users.noreply.github.com> (cherry picked from commit ce21ace)
protectionsmachine pushed a commit that referenced this pull request May 20, 2024
Co-authored-by: brokensound77 <brokensound77@users.noreply.github.com> (cherry picked from commit ce21ace)
protectionsmachine pushed a commit that referenced this pull request May 20, 2024
Co-authored-by: brokensound77 <brokensound77@users.noreply.github.com> (cherry picked from commit ce21ace)
Mikaayenson pushed a commit that referenced this pull request May 20, 2024
Co-authored-by: brokensound77 <brokensound77@users.noreply.github.com>
Mikaayenson pushed a commit that referenced this pull request May 20, 2024
Co-authored-by: brokensound77 <brokensound77@users.noreply.github.com>
Mikaayenson pushed a commit that referenced this pull request May 20, 2024
Co-authored-by: brokensound77 <brokensound77@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit. This suggestion is invalid because no changes were made to the code. Suggestions cannot be applied while the pull request is closed. Suggestions cannot be applied while viewing a subset of changes. Only one suggestion per line can be applied in a batch. Add this suggestion to a batch that can be applied as a single commit. Applying suggestions on deleted lines is not supported. You must change the existing code in this line in order to create a valid suggestion. Outdated suggestions cannot be applied. This suggestion has been applied or marked resolved. Suggestions cannot be applied from pending reviews. Suggestions cannot be applied on multi-line comments. Suggestions cannot be applied while the pull request is queued to merge. Suggestion cannot be applied right now. Please check back later.
Issues
related to #2761
Summary
A bug was unintentionally introduced in #2761 where the
test_os_and_platform_in_querywas updated to only test on rules when they arewindowsand do not includewinlogfields, ignoring all other OS's. This fixes the logic and updates some rules that merged in the mean time without thehost.os.typeset.