Skip to content

Conversation

@octoaide
Copy link
Contributor

@octoaide octoaide bot commented Dec 1, 2025

This pull request addresses issue #489 by implementing a dynamic threat_level field for all detection event structures.

Previously, threat levels for detection events used predefined values for each event type. This change introduces a threat_level: u8 field to allow Unsupervised and Supervised engines to directly assign and store the appropriate threat level for each event.

Changes Made

  • Added threat_level: u8 field to 25 detection event structures (e.g., dns.rs, http.rs, conn.rs, sysmon.rs, etc.).
  • Updated syslog_rfc5424() and Display implementations for these events to include the new threat_level.
  • Modified Match::level() to return the assigned threat_level instead of predefined values.
  • Adjusted test files and CHANGELOG.md to reflect these structural changes.

This ensures that the stored and displayed threat level accurately reflects the real-time assessment from the detection engines.

Resolves #489

@octoaide octoaide bot requested a review from syncpark December 1, 2025 19:41
@syncpark
Copy link
Contributor

syncpark commented Dec 3, 2025

The semi-supervised and unsupervised engines must first complete the task of adding the threat_level field. Until then, I will keep this blocked.

@syncpark syncpark added blocked Pending another issue and removed blocked Pending another issue labels Dec 3, 2025
@octoaide octoaide bot force-pushed the octoaide/add-threat-level-to-events branch from 55e0c90 to f15f892 Compare December 15, 2025 01:10
@octoaide
Copy link
Contributor Author

octoaide bot commented Dec 15, 2025

Thanks — I looked into this.

I rebased the branch onto main and have force-pushed the updated branch. During the rebase I resolved the merge conflicts and made the small follow-up fixes needed to keep the event types consistent (display/formatting fixes and a test expectation fix). After that, all unit and doc tests passed.

Because the branch already introduces and wires the new threat_level field across the event types and updates the related code paths, no additional changes were required in this PR to address your comment. If you were referring to external engine implementations (outside this repo) those will still need to populate threat_level on their side — but within this codebase the field has been added and integrated.

Acknowledged — I’ve addressed the integration here and force-pushed the rebased branch. Let me know if you want me to hold this until the external engines are updated or open a follow-up issue to track those engine changes.

@codecov
Copy link

codecov bot commented Dec 15, 2025

Codecov Report

❌ Patch coverage is 88.57939% with 41 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (main@6f84854). Learn more about missing BASE report.

Files with missing lines Patch % Lines
src/event/unusual_destination_pattern.rs 14.28% 6 Missing ⚠️
src/event/http.rs 84.37% 5 Missing ⚠️
src/event/conn.rs 87.09% 4 Missing ⚠️
src/event/dns.rs 88.00% 3 Missing ⚠️
src/event/ftp.rs 86.36% 3 Missing ⚠️
src/event/log.rs 50.00% 3 Missing ⚠️
src/event/network.rs 50.00% 3 Missing ⚠️
src/event/ldap.rs 90.47% 2 Missing ⚠️
src/event/rdp.rs 87.50% 2 Missing ⚠️
src/event/dcerpc.rs 87.50% 1 Missing ⚠️
... and 9 more
Additional details and impacted files
@@           Coverage Diff           @@
##             main     #637   +/-   ##
=======================================
  Coverage        ?   80.82%           
=======================================
  Files           ?       83           
  Lines           ?    23195           
  Branches        ?        0           
=======================================
  Hits            ?    18747           
  Misses          ?     4448           
  Partials        ?        0           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@octoaide
Copy link
Contributor Author

octoaide bot commented Dec 15, 2025

Hi @syncpark — quick status update and a couple options so we can move this forward.

I previously rebased this branch onto main and force-pushed a follow-up with merge-conflict resolutions and small test/display fixes. All CI and tests are passing and the PR is mergeable/clean. codecov posted a patch-coverage note listing some missing lines in a few event files (informational) but the test suite itself passed.

Context/reminder: this PR adds threat_level: u8 to all detection event structs and wires it through Match::level(), Display/syslog formatting, and tests. If your earlier "blocked" comment referred to external semi-supervised / unsupervised engines, those implementations (outside this repo) will still need to populate threat_level when they emit events. Within this repository the field has been added and integrated.

Please let me know which you prefer:

  1. If you’re happy with the changes, please approve and we can merge.
  2. If you want to hold merging until the external engine repos have been updated to populate threat_level, say so and I will hold the branch.
  3. I can also open a follow-up issue to track the work needed in external engine repos (and link it here) if you’d rather track that separately.

Which option do you prefer? If you want the follow-up issue, say so and I’ll create it and assign labels/assignees as you direct. Thanks!

@syncpark syncpark added the blocked Pending another issue label Dec 16, 2025
@syncpark
Copy link
Contributor

Adding the threat level field to all detection event structures significantly impacts not only the Manager but also the UI and the Semi-supervised/Unsupervised Engine. These programs are currently undergoing release and stabilization work, so it would be best to address this issue after stabilization is complete.

Until then, this PR will remain blocked.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

blocked Pending another issue

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add threat_level to all the detection event structures

1 participant