feat(modules/ngsiem): add LogScale/CQL query execution for NG-SIEM #204
Conversation
paulnblacklock
commented
Oct 30, 2025
paulnblacklock
commented
- Add NGSIEMModule with core query execution functionality
- Implement NGSIEMQueryEngine with extensible hook system for future enhancements
- Add NGSIEMConfig for configurable query execution parameters
- Support multiple time range formats (relative and absolute)
- Include comprehensive error handling and resource cleanup
- Add unit and e2e tests for query execution
- Update client with NG-SIEM API methods (start_search, get_search_status, stop_search)
- Extend error handling utilities for better API response management
- Add NGSIEMModule with core query execution functionality - Implement NGSIEMQueryEngine with extensible hook system for future enhancements - Add NGSIEMConfig for configurable query execution parameters - Support multiple time range formats (relative and absolute) - Include comprehensive error handling and resource cleanup - Add unit and e2e tests for query execution - Update client with NG-SIEM API methods (start_search, get_search_status, stop_search) - Extend error handling utilities for better API response management
Add API scope mappings for NGSIEM operations: - StartSearchV1: requires ngsiem:write scope - StopSearchV1: requires ngsiem:write scope - GetSearchStatusV1: requires ngsiem:read scope
carlosmmatos
requested review from
carlosmmatos,
cristianmessel-cs,
rom10 and
romanblachman
… architecture patterns - Remove start_search, get_search_status, and stop_search wrapper methods from client.py - Update ngsiem.py to use direct FalconPy command calls (StartSearchV1, GetSearchStatusV1, StopSearchV1) - Update unit tests to mock direct command calls instead of wrapper methods - Maintain all existing functionality and error handling - Achieve architectural consistency with other modules (incidents, hosts, etc.) Fixes architectural inconsistency where ngsiem module used wrapper methods while other modules call FalconPy operations directly.
protiumx
left a comment
protiumx
left a comment
There was a problem hiding this comment.
Thank you for your clean and well documented PR! I haven't gone through 100% of the changes but I have added a few main point we could resolve before a full review.
|
|
||
| # If no status_code is present, assume success (200) for responses with data | ||
| # This handles cases like NGSIEM query results that don't include status_code | ||
| if status_code is None: |
There was a problem hiding this comment.
Though I'm not fully familiar with the NG-SIEM API, this looks odd. Any HTTP response should contain a status line as per the RFC. In which cases have you noticed that this is none?
|
|
||
| # Extract resources from the response body | ||
| # Handle NGSIEM query responses - they have different structure than standard API responses | ||
| if ("search_id" in response and "results" in response) or (operation and "NG-SIEM Query" in operation): |
There was a problem hiding this comment.
I think we could instead write a special API response handler for NG-SIEM operations or accept an optional lambda function to extract its contents. I'd incline for a NG-SIEM specific handler so that we also can cover any caveat on the error handling as well.
| Attributes: | ||
| timeout_seconds: Maximum time to wait for query completion in seconds. | ||
| polling_interval: Time between status checks in seconds. |
There was a problem hiding this comment.
We might as well suffix this with seconds as we do with timeout_seconds
| self._hooks[event].append(callback) | ||
| logger.debug(f"Added hook for {event} event") | ||
| else: | ||
| logger.warning(f"Unknown hook event: {event}") |
There was a problem hiding this comment.
If this could be part of a lib, I'd suggest we either return False or throw an error. Another option would be to add an enum with the possible keys
| enable_query_templates: Whether to enable query templates. | ||
| enable_data_analysis: Whether to enable data analysis tools. | ||
| enable_field_search: Whether to enable field search helpers. | ||
| cleanup_on_timeout: Whether to cleanup searches that timeout. |
There was a problem hiding this comment.
I think this two should be a default behaviour as I don't see cases we wouldn't want to clean up the searches. We can keep this simple and always clean up on timeout or error
| return config | ||
|
|
||
|
|
||
| class NGSIEMQueryEngine: |
There was a problem hiding this comment.
I believe we could move the config and this class into a separate package, something like falcon_mcp/ng_siem or falcon_mcp/common/ng_siem.py so that the module file itself only contains the module definition. What do you think @carlosmmatos?
3 participants
