feat(arcsight-incidents): add external-import connector for ArcSight ESM cases (#6723)#6724
feat(arcsight-incidents): add external-import connector for ArcSight ESM cases (#6723)#6724SamuelHassine wants to merge 10 commits into
Conversation
…ESM cases Add a new EXTERNAL_IMPORT connector that periodically pulls ArcSight ESM cases via the Service Layer REST API (CaseService) and imports them into OpenCTI as STIX Incidents. This is the import side of a bidirectional ArcSight integration. Refs #6723
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #6724 +/- ##
===========================================
- Coverage 32.30% 0.48% -31.82%
===========================================
Files 1985 1900 -85
Lines 122106 119830 -2276
===========================================
- Hits 39441 586 -38855
- Misses 82665 119244 +36579
📢 Thoughts on this report? Let us know! 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Adds a new external-import/arcsight-incidents connector that pulls ArcSight ESM cases via the Service Layer REST API and imports them into OpenCTI as STIX 2.1 Incidents, including connector packaging (Docker/compose), metadata, and unit tests.
Changes:
- Introduces ArcSight ESM client logic (login, list case IDs, fetch cases) with retry/backoff behavior.
- Implements case → STIX Incident conversion (author identity, TLP marking, timestamps, severity, external reference) and the connector runtime loop/state handling.
- Adds connector configuration (connectors-sdk settings), generated metadata/config docs, and unit tests (settings/client/converter/connector).
Reviewed changes
Copilot reviewed 24 out of 26 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| external-import/arcsight-incidents/tests/tests_connector/test_settings.py | Settings validation tests for connectors-sdk config model. |
| external-import/arcsight-incidents/tests/tests_connector/init.py | Test package init. |
| external-import/arcsight-incidents/tests/test-requirements.txt | Test dependency definition. |
| external-import/arcsight-incidents/tests/test_main.py | Smoke tests for settings/helper/connector instantiation. |
| external-import/arcsight-incidents/tests/test_converter.py | Unit tests for case→incident conversion helpers. |
| external-import/arcsight-incidents/tests/test_connector.py | Unit tests for connector orchestration and helper interactions. |
| external-import/arcsight-incidents/tests/test_client.py | Unit tests for ArcSight API client behaviors and parsing. |
| external-import/arcsight-incidents/tests/conftest.py | Test path bootstrap to import connector modules. |
| external-import/arcsight-incidents/tests/init.py | Test package init. |
| external-import/arcsight-incidents/src/requirements.txt | Connector runtime dependencies. |
| external-import/arcsight-incidents/src/main.py | Connector entrypoint wiring settings/helper/connector. |
| external-import/arcsight-incidents/src/connector/settings.py | connectors-sdk based settings models for this connector. |
| external-import/arcsight-incidents/src/connector/converter_to_stix.py | ArcSight case → STIX 2.1 Incident conversion. |
| external-import/arcsight-incidents/src/connector/connector.py | Main connector loop: collect, bundle, send, and persist state. |
| external-import/arcsight-incidents/src/connector/init.py | Package exports for connector components. |
| external-import/arcsight-incidents/src/config.yml.sample | Sample YAML configuration for deployment. |
| external-import/arcsight-incidents/src/arcsight_client/api_client.py | HTTP client for ArcSight LoginService/CaseService endpoints. |
| external-import/arcsight-incidents/src/arcsight_client/init.py | Package export for ArcSight client. |
| external-import/arcsight-incidents/README.md | Usage and configuration documentation. |
| external-import/arcsight-incidents/entrypoint.sh | Container entrypoint. |
| external-import/arcsight-incidents/Dockerfile | Container build for the connector. |
| external-import/arcsight-incidents/docker-compose.yml | Example docker-compose deployment. |
| external-import/arcsight-incidents/.dockerignore | Docker build context exclusions. |
| external-import/arcsight-incidents/metadata/connector_manifest.json | Connector manifest metadata for registry/manager. |
| external-import/arcsight-incidents/metadata/connector_config_schema.json | Generated JSON schema for config variables. |
| external-import/arcsight-incidents/metadata/CONNECTOR_CONFIG_DOC.md | Generated config documentation. |
ArcSight cases are case-management artifacts, so they must map to OpenCTI Case-Incidents (CustomObjectCaseIncident) rather than Incidents, which are reserved for alerts/detections. Add severity-based priority. Refs #6723
…e-Incidents (#6723) ArcSight exposes two distinct concepts. Model them as two STIX entities: security events referenced by a case become STIX Incidents, and the case itself becomes a STIX Case-Incident that references those Incidents through object_refs. Adds get_case_events (SecurityEventService) to the client, create_incident to the converter, and a dual collection loop. Tests and docs updated.
Addresses review findings on the new ArcSight Incidents connector:
- converter: tlp_level "clear" now emits a distinct TLP:CLEAR custom
marking instead of aliasing STIX TLP:WHITE.
- api_client: _request retries only on connection/timeout errors, rate
limiting (429) and server errors (5xx); other 4xx responses fail fast,
letting the caller re-issue the auth token immediately. Request
failures are logged via meta={...}.
- docs: CONNECTOR_SCOPE is required (the generated schema lists it as
required) - fixed config.yml.sample, docker-compose.yml and README; the
config.yml.sample tlp_level comment now lists "white" too.
- tests: settings test asserts on str(err.value); fixed a grammar typo in
the tests requirements comment.
Tests added for the TLP:CLEAR marking and the 4xx/5xx request behavior.
- docker-compose.yml now uses the "ChangeMe" placeholder consistently, matching config.yml.sample and the rest of the repo's connectors. - README: the CONNECTOR_LOG_LEVEL row now documents the "warning" level, which the generated config schema already enumerates (alongside "warn").
Stop logging str(err) in _request: the ESM LoginService login call carries the password as a query parameter (and findAllIds/getResourceById carry the auth token), and a requests exception string usually embeds the full request URL, leaking those secrets into the connector logs. Errors now log only the path, the status code and the exception type. Also align the _load_config_dict test overrides with the SDK (-> Self instead of dict[str, Any]). Adds tests asserting the exception string is never logged.
Review-and-fix pass summaryIndependent senior review of the full new connector plus all 5 remaining open Copilot threads.
Verified locally: 45/45 unit tests pass (added 2 redaction tests); Remaining (non-CI) blocker: |
… on error Address the open Copilot review threads plus an independent senior review. - converter_to_stix.py: Incident.generate_id and CaseIncident.generate_id are now seeded with the source timestamp only. _to_iso is split into _parse_timestamp (returns None on a missing or unparseable timestamp) and _format_iso; with no usable timestamp the STIX created still falls back to "now" but the id seed is None, so re-importing an event or case keeps a stable id instead of creating a duplicate Incident / Case-Incident every run. - connector.py: the initiated work is finalized with in_error=True on the failure path, so a failed run no longer leaves a dangling in-progress work in OpenCTI. - tests: cover the deterministic id for both the missing- and unparseable-timestamp cases (Incident and Case-Incident), the scalar baseEventIds branch, and the in-error work finalize; the converter module is now 100% covered.
Second review-and-fix pass summaryIndependent senior re-review of the full Code fixes (commit 67fe59b):
Also verified the connector does not have the data-loss pattern from the sibling fortisiem-incidents connector: Tests: added the deterministic id (missing and unparseable timestamp, Incident and Case-Incident), the scalar Status: all CI checks are green (codecov/patch and codecov/project, target 80%), Remaining (non-CI) blocker: |
Seed Incident.generate_id / CaseIncident.generate_id with the event id / case external id (not just the display name), so distinct events or cases that happen to share a name no longer collapse into one entity. When the source carries no usable timestamp, created/modified now fall back to a fixed sentinel instead of "now", so a re-imported event/case keeps a stable id and is not re-sent with drifting created/modified (needless updates) every run. Covered by new converter tests for the distinct-id and stable-timestamp behavior of both incidents and case-incidents.
Review-and-fix pass summaryIndependent senior re-review of the full
Remaining (non-CI) blocker: |
Address the remaining Copilot review plus an independent senior pass, and keep the connector installable against current master: - converter_to_stix.py: the Case-Incident `modified` now falls back to the already-stable `created` value (never "now") when the ArcSight modified timestamp is missing or unparseable, so the deterministic id is no longer re-sent with a drifting `modified` each run. - connector.py: the top-level error handler no longer logs or forwards `str(err)` (the ESM auth flow carries the password/token in query parameters, which a requests exception string can embed); it logs the exception type via structured `meta` and sends a sanitized work message. - arcsight_client/api_client.py: `_request` closes the HTTP response on the 429/5xx-retry and non-retriable HTTPError paths so a periodic run does not keep connections checked out of the requests Session pool. - src/requirements.txt: bump the pinned pycti to 7.260615.0 to match the current connectors-sdk; align the README minimum and the manifest support_version.
…cidents-external-import
Review-and-fix pass summaryIndependent senior re-review of the full Code/test fixes (commit 6c80ab1):
CI fix (de-stale):
Verification:
Remaining (non-CI) blocker: |
There was a problem hiding this comment.
⚠️ Human review recommended
It introduces a full new connector (API client + STIX modeling + deployment artifacts), which warrants a final maintainer review despite the strong unit-test coverage.
Copilot's findings
- Files reviewed: 24/26 changed files
- Comments generated: 0 new
Note
Your feedback helps us improve the quality of this feature.
Please use 👍 or 👎 to tell us whether this assessment is correct.
Proposed changes
This PR adds a new
EXTERNAL_IMPORTconnectorexternal-import/arcsight-incidentsthat imports ArcSight ESM cases into OpenCTI as STIX Incidents and Case-Incidents. It is the import side of a bidirectional ArcSight integration (paired with thestream/arcsightconnector that pushes IOCs to ESM Active Lists).LoginService, lists case ids viaCaseService(capped per run) and fetches each case and its security events.object_refs), attributed to an ArcSight author identity and marked with a configurable TLP.connectors-sdksettings pattern, with unit tests and connector metadata (manifest, config schema, configuration documentation).Related issues
Closes #6723
Type of change
Maintainer review (independent review-and-fix)
Independent senior review of the full connector across several passes, on top of the Copilot threads. Substantive fixes:
arcsight_client/api_client.py_requestnever logsstr(err). The ESMLoginServicelogin carries the password as a query parameter (andfindAllIds/getResourceByIdcarry the auth token), and arequestsexception string usually embeds the full request URL, sostr(err)could leak those secrets. Errors now log onlypath+status_code+ exception type.connector.pytop-level error handler no longer logs or forwardsstr(err)either; it logs the exception type via structuredmetaand finalizes the work with a sanitized message (ArcSight Incidents connector run failed: <ExceptionType>), so a requests exception URL carrying the password/token cannot leak into the logs or the work message.converter_to_stix.py:tlp_level="clear"emits a dedicated TLP:CLEAR custom statement marking (x_opencti_definition="TLP:CLEAR") instead of aliasingstix2.TLP_WHITE;amber+strictlikewise emits the OpenCTI-specific marking.converter_to_stix.py(deterministic ids):Incident.generate_id/CaseIncident.generate_idare seeded with the event id / case external id in the name (so distinct events or cases that share a display name no longer collapse into one entity), and when the source carries no usable timestampcreated/modifiedfall back to a fixed sentinel instead of "now". The Case-Incidentmodifiednow also falls back to the stablecreated(never "now") when the modified timestamp is missing or unparseable, so a re-imported case keeps a stable id and is not re-sent with a driftingmodifiedeach run.api_client.py:_requestretries only on connection/timeout errors, 429 and 5xx; other 4xx (401/403/404) fail fast. It now also closes the HTTP response on the 429/5xx-retry and non-retriable HTTPError paths, so a periodic run does not keep connections checked out of the requests Session pool.connector.py: the initiated work is finalized within_error=Trueon the failure path, so a failed run does not leave a dangling in-progress work.config.yml.sample/docker-compose.yml/README.mdmarkscope/CONNECTOR_SCOPEas required (matching the SDK base settings and the generated schema), thetlp_level/ log-level option lists are in sync, the settings test asserts onstr(err.value), and a grammar typo intests/test-requirements.txtis fixed.src/requirements.txt/README.md/__metadata__/connector_manifest.json: the pinnedpyctiis bumped to7.260615.0to match the currentconnectors-sdk, with the README minimum and manifestsupport_versionaligned. The branch was merged with currentmasterto de-stale, so the localconnectors-sdkused byrun_test.shpins the samepyctiand the test environment resolves consistently.Decisions (Copilot comment intentionally not applied)
apk update && apk upgrade && apk --no-cache add ...line is kept: it is the shared pattern across the sibling connectors (corelight-investigator,fortisiem-incidents,sentinelone-intel,infoblox-threat-defense); thegit/build-basebuild deps are pruned afterwards withapk del, and thelibmagic/libffi-dev/libxml2-dev/libxslt-devpackages back pycti's transitive C-extension deps. Changing only this connector would be a one-off and risk breaking the image build; a leaner shared base image is better handled repo-wide.Tests
Unit tests cover the settings model (validation, defaults), the ArcSight client (auth, case listing/fetch, 4xx fail-fast vs 429/5xx retry, credential-redaction in logs), the converter (TLP:CLEAR marking, severity mapping, deterministic and collision-free incident/case ids, stable fallback timestamps, modified-falls-back-to-created) and the connector (work finalized
in_erroron failure). 54 unit tests pass;black/isort/flake8 --ignore=E,Wclean locally.Checklist
Status
All CI checks are green (tests, lint/format, STIX ID linter,
codecov/patchandcodecov/project) and there are 0 unresolved review threads.mergeStateStatusis BLOCKED only becausereviewDecisionisREVIEW_REQUIRED- the PR needs one approving review from a maintainer other than me (as the author I cannot self-approve). The branch carries a de-stale merge commit, so it should be squash-merged.