Add logging support for ES_EVENT_TYPE_NOTIFY_PROC_SUSPEND_RESUME#883
Open
pmarkowsky wants to merge 5 commits intonorthpolesec:mainfrom
Open
Add logging support for ES_EVENT_TYPE_NOTIFY_PROC_SUSPEND_RESUME#883pmarkowsky wants to merge 5 commits intonorthpolesec:mainfrom
pmarkowsky wants to merge 5 commits intonorthpolesec:mainfrom
Conversation
Add full pipeline support to log process suspend/resume events: - Proto: ProcSuspendResume message with instigator, target, and Type enum - Enriched type: EnrichedProcSuspendResume with optional target process - Telemetry: kProcSuspendResume mapping so events pass ShouldLog() gate - Enricher: Handle NOTIFY_PROC_SUSPEND_RESUME with nullable target - Serializers: Protobuf, BasicString, and Empty implementations - Recorder: Subscribe to the NOTIFY event - Metrics: EventTypeToString for the NOTIFY variant
github-actions
bot
added
docs
Contributor
📝 WalkthroughWalkthroughAdds support for a new PROC_SUSPEND_RESUME event across telemetry, ES mapping, enrichment, protobuf, recorder subscription, serializers (BasicString/Protobuf/Empty), metrics, tests, and config/test fixtures. Changes
Sequence Diagram(s)sequenceDiagram
participant ES as EndpointSecurity
participant Rec as SNTEndpointSecurityRecorder
participant Enr as Enricher
participant Ser as Serializer
participant Sink as Telemetry/Storage
ES->>Rec: notify ES_EVENT_TYPE_NOTIFY_PROC_SUSPEND_RESUME
Rec->>Enr: forward raw ES Message
Enr->>Enr: enrich instigator (and optional target)
Enr->>Ser: Serialize EnrichedProcSuspendResume
Ser->>Sink: emit bytes (log/protobuf/telemetry)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
Source/common/TelemetryEventMapTest.mm (1)
115-115: Add config-key coverage forProcSuspendResumetoo.Great that ES mapping is tested; please also add a
TelemetryConfigToBitmaskcase for the new string key so both mapping paths are covered.✅ Suggested test addition
std::map<std::string_view, TelemetryEvent> eventNameToMask = { {"ExeCUTion", TelemetryEvent::kExecution}, @@ {"LaunchItem", TelemetryEvent::kLaunchItem}, {"TCCModification", TelemetryEvent::kTCCModification}, {"XProtect", TelemetryEvent::kXProtect}, + {"ProcSuspendResume", TelemetryEvent::kProcSuspendResume}, // special cases {"none", TelemetryEvent::kNone},🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Source/common/TelemetryEventMapTest.mm` at line 115, Add a unit test case to cover the config-key path for the new ProcSuspendResume mapping: add a TelemetryConfigToBitmask test that uses the string key corresponding to TelemetryEvent::kProcSuspendResume (the same key added for ES_EVENT_TYPE_NOTIFY_PROC_SUSPEND_RESUME) and assert the returned bitmask includes the TelemetryEvent::kProcSuspendResume bit; update or add the test in TelemetryEventMapTest to exercise TelemetryConfigToBitmask with that string key so both mapping paths (ES_EVENT_TYPE_NOTIFY_PROC_SUSPEND_RESUME -> TelemetryEvent::kProcSuspendResume and config-key -> TelemetryEvent::kProcSuspendResume) are covered.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@Source/common/TelemetryEventMapTest.mm`:
- Line 115: Add a unit test case to cover the config-key path for the new
ProcSuspendResume mapping: add a TelemetryConfigToBitmask test that uses the
string key corresponding to TelemetryEvent::kProcSuspendResume (the same key
added for ES_EVENT_TYPE_NOTIFY_PROC_SUSPEND_RESUME) and assert the returned
bitmask includes the TelemetryEvent::kProcSuspendResume bit; update or add the
test in TelemetryEventMapTest to exercise TelemetryConfigToBitmask with that
string key so both mapping paths (ES_EVENT_TYPE_NOTIFY_PROC_SUSPEND_RESUME ->
TelemetryEvent::kProcSuspendResume and config-key ->
TelemetryEvent::kProcSuspendResume) are covered.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d3f1d642-22be-462a-a0cc-7cd0f8954c82
📒 Files selected for processing (17)
Source/common/TelemetryEventMap.hSource/common/TelemetryEventMap.mmSource/common/TelemetryEventMapTest.mmSource/common/es/EnrichedTypes.hSource/common/es/Enricher.mmSource/common/santa.protoSource/santad/EventProviders/SNTEndpointSecurityRecorder.mmSource/santad/EventProviders/SNTEndpointSecurityRecorderTest.mmSource/santad/Logs/EndpointSecurity/Serializers/BasicString.hSource/santad/Logs/EndpointSecurity/Serializers/BasicString.mmSource/santad/Logs/EndpointSecurity/Serializers/Empty.hSource/santad/Logs/EndpointSecurity/Serializers/Empty.mmSource/santad/Logs/EndpointSecurity/Serializers/Protobuf.hSource/santad/Logs/EndpointSecurity/Serializers/Protobuf.mmSource/santad/Logs/EndpointSecurity/Serializers/Serializer.hSource/santad/Metrics.mmdocs/src/lib/santaconfig.ts
Resolve conflicts keeping ProcSuspendResume additions with main's formatting style.
mlw
requested changes
Mar 29, 2026
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
Source/santad/Logs/EndpointSecurity/Serializers/ProtobufTest.mm (1)
1568-1579: Consider adding fixture coverage for nullabletargetand anothertypevariant.This test validates only
TYPE_SUSPENDwith a populated target. Adding at least onetarget = NULLcase and one non-suspend type would better lock in the new serializer branches.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Source/santad/Logs/EndpointSecurity/Serializers/ProtobufTest.mm` around lines 1568 - 1579, The current test testSerializeMessageProcSuspendResume only covers ES_PROC_SUSPEND_RESUME_TYPE_SUSPEND with a non-null target; add coverage for the serializer branches by adding at least two additional cases: one where esMsg->event.proc_suspend_resume.target = NULL and one where esMsg->event.proc_suspend_resume.type is set to a non-suspend variant (e.g., ES_PROC_SUSPEND_RESUME_TYPE_RESUME or another enum value) and then call the same helper ([self serializeAndCheckEvent:messageSetup:]) to assert correct serialization for those paths; locate the test helper usages and es_msg population in testSerializeMessageProcSuspendResume to copy the pattern and create the new test methods (or parametrized subcases) validating null target and alternate type handling.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@Source/santad/Logs/EndpointSecurity/Serializers/ProtobufTest.mm`:
- Around line 1568-1579: The current test testSerializeMessageProcSuspendResume
only covers ES_PROC_SUSPEND_RESUME_TYPE_SUSPEND with a non-null target; add
coverage for the serializer branches by adding at least two additional cases:
one where esMsg->event.proc_suspend_resume.target = NULL and one where
esMsg->event.proc_suspend_resume.type is set to a non-suspend variant (e.g.,
ES_PROC_SUSPEND_RESUME_TYPE_RESUME or another enum value) and then call the same
helper ([self serializeAndCheckEvent:messageSetup:]) to assert correct
serialization for those paths; locate the test helper usages and es_msg
population in testSerializeMessageProcSuspendResume to copy the pattern and
create the new test methods (or parametrized subcases) validating null target
and alternate type handling.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7da61dab-81f7-4c26-a137-9322f5d4869b
📒 Files selected for processing (9)
Source/common/TelemetryEventMapTest.mmSource/santad/Logs/EndpointSecurity/Serializers/BasicString.mmSource/santad/Logs/EndpointSecurity/Serializers/BasicStringTest.mmSource/santad/Logs/EndpointSecurity/Serializers/ProtobufTest.mmSource/santad/testdata/protobuf/v4/proc_suspend_resume.jsonSource/santad/testdata/protobuf/v5/proc_suspend_resume.jsonSource/santad/testdata/protobuf/v6/proc_suspend_resume.jsonSource/santad/testdata/protobuf/v7/proc_suspend_resume.jsonSource/santad/testdata/protobuf/v8/proc_suspend_resume.json
✅ Files skipped from review due to trivial changes (6)
- Source/santad/testdata/protobuf/v8/proc_suspend_resume.json
- Source/santad/testdata/protobuf/v5/proc_suspend_resume.json
- Source/santad/testdata/protobuf/v7/proc_suspend_resume.json
- Source/santad/testdata/protobuf/v4/proc_suspend_resume.json
- Source/santad/testdata/protobuf/v6/proc_suspend_resume.json
- Source/santad/Logs/EndpointSecurity/Serializers/BasicString.mm
🚧 Files skipped from review as they are similar to previous changes (1)
- Source/common/TelemetryEventMapTest.mm
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 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.
Add full support for logging process suspend/resume events:
ProcSuspendResumemessage with instigator, target, and Type enumkProcSuspendResumeNOTIFY_PROC_SUSPEND_RESUMEevent