feat(paloalto): create cortex XSOAR collector (#301)#348
Conversation
6cf130c to
11e31c3
Compare
|
Unless I'm mistaken, the API rate-limit responses leading to a backoff and retry is missing from the codebase (US.3 Error Handling & Resilience, AC-3.1, part of chunk 5 Hardening & Delivery). Edit: handled by including the HTTP error 429 in the retry/backoff system |
87fe716 to
1caf077
Compare
Kakudou
left a comment
There was a problem hiding this comment.
Mostly QoL, so i will conduct the execution/usage tests of the collector while waiting for those small fixes ;)
31e9405 to
715f894
Compare
715f894 to
47334eb
Compare
67ff32e to
f3c1f11
Compare
There was a problem hiding this comment.
Pull request overview
Adds a new standalone Python collector (palo-alto-cortex-xsoar) to query Cortex XSOAR incidents, match them against OpenAEV expectations (detection/prevention), and report results + traces back to OpenAEV, including Docker packaging and CircleCI integration.
Changes:
- Introduces the Palo Alto Cortex XSOAR collector implementation (config loading, API client, alert fetching, expectation matching, trace creation/submission).
- Adds a full pytest suite (factories/fixtures + unit tests) for the new collector.
- Wires the collector into build/test/release assets (Dockerfiles, compose example, CircleCI steps, metadata, README).
Reviewed changes
Copilot reviewed 48 out of 51 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| palo-alto-cortex-xsoar/tests/test_trace_service.py | Tests for trace creation logic |
| palo-alto-cortex-xsoar/tests/test_trace_manager_extra.py | Extra tests for trace submission paths |
| palo-alto-cortex-xsoar/tests/test_trace_builder.py | Tests for trace builder utilities |
| palo-alto-cortex-xsoar/tests/test_expectation_service.py | Tests for expectation processing service |
| palo-alto-cortex-xsoar/tests/test_expectation_manager_extra.py | Extra tests for expectation manager behaviors |
| palo-alto-cortex-xsoar/tests/test_converter_and_extractor.py | Tests for converter + signature extraction helpers |
| palo-alto-cortex-xsoar/tests/test_collector.py | Higher-level collector behavior tests |
| palo-alto-cortex-xsoar/tests/test_collector_extra.py | Additional collector error-path tests |
| palo-alto-cortex-xsoar/tests/test_client_api.py | Tests for XSOAR HTTP API client |
| palo-alto-cortex-xsoar/tests/test_authentication.py | Tests for auth header generation |
| palo-alto-cortex-xsoar/tests/test_alert_fetcher.py | Tests for alert fetching + pagination |
| palo-alto-cortex-xsoar/tests/factories.py | Factory-boy fixtures for expectations/incidents/alerts |
| palo-alto-cortex-xsoar/tests/conftest.py | Shared pytest fixtures/mocks |
| palo-alto-cortex-xsoar/tests/init.py | Test package marker |
| palo-alto-cortex-xsoar/src/services/utils/trace_builder.py | Builds trace dictionaries from incidents |
| palo-alto-cortex-xsoar/src/services/utils/signature_extractor.py | Extracts/groups expectation signatures |
| palo-alto-cortex-xsoar/src/services/utils/init.py | Exports service utilities |
| palo-alto-cortex-xsoar/src/services/trace_service.py | Converts match results into OpenAEV trace models |
| palo-alto-cortex-xsoar/src/services/expectation_service.py | Orchestrates fetch/match/result creation |
| palo-alto-cortex-xsoar/src/services/exception.py | Service-layer exception types |
| palo-alto-cortex-xsoar/src/services/converter.py | Converts incident data into OAEV matching format |
| palo-alto-cortex-xsoar/src/services/client_api.py | Requests session + retrying XSOAR API client |
| palo-alto-cortex-xsoar/src/services/alert_fetcher.py | Time-window search + pagination fetcher |
| palo-alto-cortex-xsoar/src/services/init.py | Service package marker |
| palo-alto-cortex-xsoar/src/models/settings/palo_alto_cortex_xsoar_configs.py | Pydantic settings for XSOAR config |
| palo-alto-cortex-xsoar/src/models/settings/config_loader.py | Loads config from env/yaml/dotenv and flattens to daemon config |
| palo-alto-cortex-xsoar/src/models/settings/collector_configs.py | Collector/OpenAEV base settings models |
| palo-alto-cortex-xsoar/src/models/settings/base_settings.py | Shared BaseSettings configuration |
| palo-alto-cortex-xsoar/src/models/settings/init.py | Exports settings models |
| palo-alto-cortex-xsoar/src/models/incident.py | Pydantic models for XSOAR incident payloads |
| palo-alto-cortex-xsoar/src/models/authentication.py | Standard/advanced authentication header builder |
| palo-alto-cortex-xsoar/src/models/init.py | Exports top-level models |
| palo-alto-cortex-xsoar/src/config.yml.sample | Sample YAML configuration |
| palo-alto-cortex-xsoar/src/collector/trace_manager.py | Submits traces (bulk + fallback) |
| palo-alto-cortex-xsoar/src/collector/models.py | Pydantic models for results/traces/summary |
| palo-alto-cortex-xsoar/src/collector/expectation_manager.py | Fetch/process/update expectations and trigger tracing |
| palo-alto-cortex-xsoar/src/collector/exception.py | Collector-layer exception types |
| palo-alto-cortex-xsoar/src/collector/collector.py | CollectorDaemon integration + processing loop |
| palo-alto-cortex-xsoar/src/collector/init.py | Exports Collector |
| palo-alto-cortex-xsoar/src/main.py | Module entrypoint |
| palo-alto-cortex-xsoar/src/init.py | Package init/export |
| palo-alto-cortex-xsoar/README.md | Collector documentation and run/config instructions |
| palo-alto-cortex-xsoar/pyproject.toml | Collector packaging + dependencies |
| palo-alto-cortex-xsoar/manifest-metadata.json | Marketplace/manifest metadata |
| palo-alto-cortex-xsoar/Dockerfile_ubi9 | UBI9 container build |
| palo-alto-cortex-xsoar/Dockerfile | Alpine container build |
| palo-alto-cortex-xsoar/docker-compose.yml | Local compose example |
| palo-alto-cortex-xsoar/.python-version | Local Python version pin |
| palo-alto-cortex-xsoar/.gitignore | Collector-local ignores |
| .circleci/config.yml | CI updates for tests + docker builds/push |
| PaloAltoCortexXSOARNetworkError, | ||
| PaloAltoCortexXSOARValidationError, | ||
| ) | ||
| from src.services.ioc_extractor import IncidentResult, extract_from_custom_fields | ||
|
|
| except Exception as e: | ||
| self.logger.error(f"{LOG_PREFIX} Bulk trace submission failed: {e}") | ||
| try: | ||
| self.logger.info( | ||
| f"{LOG_PREFIX} Attempting individual trace creation as fallback..." | ||
| ) | ||
| self._fallback_individual_trace_creation(traces) | ||
| except TraceCreationError as fallback_error: | ||
| self.logger.error( | ||
| f"{LOG_PREFIX} Fallback trace creation also failed: {fallback_error}" | ||
| ) | ||
| raise TraceSubmissionError(f"Error submitting traces: {e}") from e | ||
|
|
| def test_submit_traces_fallback_success(manager, mock_oaev_api): | ||
| mock_trace = MagicMock() | ||
| mock_trace.to_api_dict.return_value = {"key": "val"} | ||
| mock_oaev_api.inject_expectation_trace.bulk_create.side_effect = Exception( | ||
| "bulk fail" | ||
| ) | ||
|
|
||
| with patch.object(manager, "_fallback_individual_trace_creation") as mock_fallback: | ||
| with pytest.raises(TraceSubmissionError): | ||
| manager._submit_traces([mock_trace]) | ||
| mock_fallback.assert_called_once_with([mock_trace]) | ||
|
|
| RUN if [[ ${PYOAEV_GIT_BRANCH_OVERRIDE} ]] ; then \ | ||
| echo "Forcing specific version of client-python" && \ | ||
| apk add --no-cache git && \ | ||
| pip install pip3-autoremove && \ | ||
| pip-autoremove pyoaev -y && \ | ||
| pip install git+https://github.com/OpenAEV-Platform/client-python@${PYOAEV_GIT_BRANCH_OVERRIDE} ; \ | ||
| fi |
| | Parameter | Env Variable | Description | Default | | ||
| |-----------------------------------------|------------------------------------------|--------------------------------------------------|--------------| | ||
| | `palo_alto_cortex_xsoar.api_url` | `PALO_ALTO_CORTEX_XSOAR_API_URL` | XSOAR tenant API URL (without `https://`) | *(required)* | | ||
| | `palo_alto_cortex_xsoar.api_key` | `PALO_ALTO_CORTEX_XSOAR_API_KEY` | API Key for authentication | *(required)* | | ||
| | `palo_alto_cortex_xsoar.api_key_id` | `PALO_ALTO_CORTEX_XSOAR_API_KEY_ID` | API Key ID for authentication | *(required)* | | ||
| | `palo_alto_cortex_xsoar.api_key_type` | `PALO_ALTO_CORTEX_XSOAR_API_KEY_TYPE` | Key type: `standard` or `advanced` | `standard` | | ||
| | `palo_alto_cortex_xsoar.time_window` | `PALO_ALTO_CORTEX_XSOAR_TIME_WINDOW` | Default time window for incident searches | `1 hour` | | ||
|
|
| palo_alto_cortex_xsoar: | ||
| api_url: "api-example.xsoar.fa.paloaltonetworks.com" | ||
| api_key: "ChangeMe" | ||
| api_key_id: "ChangeMe" | ||
| api_key_type: "standard" # standard or advanced | ||
| ``` |
| def test_data_conversion_error_reraised(self, service): | ||
| result = ExpectationResult( | ||
| expectation_id="exp-1", | ||
| is_valid=True, | ||
| matched_alerts=[{"alert_name": "Alert", "alert_link": "http://link"}], | ||
| ) | ||
| with patch.object( | ||
| service, | ||
| "_create_expectation_trace", | ||
| side_effect=PaloAltoCortexXSOARDataConversionError("conversion fail"), | ||
| ): | ||
| # The DataConversionError from inside the loop is caught by the generic except, | ||
| # but the outer except re-raises it | ||
| # Actually the inner loop catches generic Exception, so it won't propagate. | ||
| # Let's force it differently by patching at a higher level | ||
| pass | ||
|
|
||
| # Force re-raise of DataConversionError from the outer try | ||
| with patch( | ||
| "src.services.trace_service.TraceService._create_expectation_trace", | ||
| ) as mock_create: | ||
| mock_create.return_value = MagicMock() | ||
| # This should work fine | ||
| traces = service.create_traces_from_results([result], "collector-1") | ||
| assert len(traces) == 1 |
This reverts commit 67ff32e.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
…ocal and UTC datetimes
0590099 to
deead5a
Compare
deead5a to
4236506
Compare
Kakudou
left a comment
There was a problem hiding this comment.
Review Finding
During my review, I noticed a misbehavior in the current XSOAR incident retrieval and alert matching logic.
The endpoint we use retrieves Incidents, named cases in the UI. Those incidents are containers filled with Alerts.
So far, we filter incidents using fromDate and toDate.
Currently, fromDate is computed with:
start_time = end_time - self.time_windowwhere end_time is the expectation signature end_date, and self.time_window comes from the time_window setting:
Time window for PaloAltoCortexXSOAR alert searches when no date signature are provided (ISO 8601 format).
This creates a default incident retrieval window of 1h, which can be far too broad and noisy.
Required Fix
Use the expectation signature dates directly when available:
fromDate = start_date
toDate = end_dateFallback to:
fromDate = now() - self.time_window
toDate = now()only when no date signature is provided at all.
Detection Logic Change
start_date and end_date must now be treated as a weak but valid signature.
IP and ProcessName are deterministic enough in theory, but not reliable enough in the current XSOAR context. We will think later, in a future collector iteration, about more suitable signatures.
For now, after retrieving incidents, we must inspect the alerts contained inside each incident and keep only alerts whose own timestamp is inside the expectation time window.
If at least one alert exists inside [start_date, end_date], then:
is_detected = Trueeven if no other signature matches.
Then is_prevented must be evaluated from the matched alert status only.
Alerts outside the expectation time window must be ignored.
Checklist
- Use expectation
start_date/end_datefor incident retrieval. - Use
settings.time_windowonly as fallback when no date signature exists. - Filter alerts inside each incident by alert timestamp.
- Ignore alerts outside
[start_date, end_date]. - Set
is_detected = Truewhen at least one alert exists in the time window. - Derive
is_preventedonly from the matched alert status. - Keep
IP/ProcessNameas useful signatures, but not mandatory in XSOAR for detection matching, if not present, fallback to solelystart/end_datematching.
Proposed changes
Testing Instructions
Related issues
Checklist
Further comments