feat(arcsight): add stream connector to sync IOCs to ArcSight ESM active lists (#6717)#6718
feat(arcsight): add stream connector to sync IOCs to ArcSight ESM active lists (#6717)#6718SamuelHassine wants to merge 7 commits into
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #6718 +/- ##
===========================================
- Coverage 32.30% 0.35% -31.95%
===========================================
Files 1985 1900 -85
Lines 122106 119660 -2446
===========================================
- Hits 39444 427 -39017
- Misses 82662 119233 +36571
📢 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 stream connector under stream/arcsight to synchronize OpenCTI live-stream indicators (IOC values extracted from STIX patterns) into an ArcSight ESM Active List via the Service Layer REST API, including connector packaging assets, metadata, and tests.
Changes:
- Introduces ArcSight ESM API client with login/token caching and add/delete entry operations for Active Lists.
- Adds stream connector implementation + Pydantic-based settings using
connectors-sdk. - Adds unit tests plus deployment/documentation artifacts (Dockerfile, compose, README, metadata, config schema/docs).
Reviewed changes
Copilot reviewed 23 out of 25 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| stream/arcsight/src/arcsight_client/api_client.py | Implements ArcSight Service Layer REST client (login + Active List entry add/delete). |
| stream/arcsight/src/arcsight_client/stix_patterns.py | Extracts observable values from supported single-observable STIX patterns. |
| stream/arcsight/src/arcsight_client/init.py | Exposes ArcSight client package API. |
| stream/arcsight/src/connector/connector.py | Stream connector wiring: consumes live stream messages and calls ArcSight client. |
| stream/arcsight/src/connector/settings.py | connectors-sdk settings models for STREAM connector + ArcSight-specific config. |
| stream/arcsight/src/connector/init.py | Exports connector public API. |
| stream/arcsight/src/main.py | Runtime entrypoint to build settings/helper and start connector. |
| stream/arcsight/src/requirements.txt | Connector runtime dependencies. |
| stream/arcsight/src/config.yml.sample | Sample YAML configuration for local/manual deployment. |
| stream/arcsight/tests/test_client.py | Unit tests for ArcSight client behavior (token caching, retry, add/delete). |
| stream/arcsight/tests/test_connector.py | Unit tests for connector message handling and stream listening behavior. |
| stream/arcsight/tests/test_main.py | Smoke tests for settings/helper/connector instantiation with mocked pycti internals. |
| stream/arcsight/tests/tests_connector/test_settings.py | Unit tests for settings validation and defaults. |
| stream/arcsight/tests/tests_connector/init.py | Marks tests package (empty init). |
| stream/arcsight/tests/conftest.py | Ensures src/ is importable in the test environment. |
| stream/arcsight/tests/test-requirements.txt | Test dependencies for isolated connector test runs. |
| stream/arcsight/tests/init.py | Marks tests package (empty init). |
| stream/arcsight/Dockerfile | Container build for the ArcSight connector. |
| stream/arcsight/entrypoint.sh | Container entrypoint script. |
| stream/arcsight/docker-compose.yml | Example docker-compose deployment configuration. |
| stream/arcsight/.dockerignore | Docker build context exclusions. |
| stream/arcsight/README.md | Connector documentation (setup, configuration, behavior, supported observables). |
| stream/arcsight/metadata/connector_manifest.json | Connector manifest metadata for OpenCTI connectors catalog. |
| stream/arcsight/metadata/connector_config_schema.json | Generated config schema for environment variables. |
| stream/arcsight/metadata/CONNECTOR_CONFIG_DOC.md | Generated configuration documentation from schema. |
Review-and-fix passIndependent senior review of the full new connector plus the 2 Copilot threads (both valid; both fixed). Fixes:
Verified: 31/31 unit tests pass;
|
Review-and-fix pass summaryIndependent senior re-review of the full new connector plus the 3 remaining open Copilot threads, fixed in
Verified locally: 40/40 unit tests pass (added redaction + stream-id placeholder tests); Note: Remaining (non-CI) blocker: |
Third review-and-fix pass summaryIndependent senior re-review of the full Code fix (commit bd8d17a):
Re-reviewed the rest file by file with no further findings: the ArcSight client redacts credentials from logs (it logs only Tests: added Status: all CI checks are green (codecov/patch and codecov/project, target 80%), Remaining (non-CI) blocker: |
Review-and-fix pass summaryIndependent senior re-review of the full
Remaining (non-CI) blocker: |
Review-and-fix pass summaryIndependent senior re-review of the full Code/test fixes (commit 26ca44f):
CI fix (de-stale):
Verification:
Remaining (non-CI) blocker: |
…ive lists Add a new STREAM connector that listens to an OpenCTI live stream and adds/removes IOC values to an ArcSight ESM Active List via the ESM Service Layer REST API (LoginService + ActiveListService). Refs #6717
…test
Addresses review findings on the new ArcSight stream connector:
- process_message catches only the expected parse errors
(json.JSONDecodeError / KeyError / TypeError) and chains via
`raise ... from err` to preserve the root-cause traceback.
- _request() retries only on connection/timeout errors, rate limiting
(429) and server errors (5xx); other 4xx responses (401/403/404) now
fail fast. This also lets _post_entries trigger a token re-issue
immediately on a 401 instead of after three backoff sleeps. Request
failures are logged via meta={...}.
- test_settings asserts on str(err.value) (the actual exception message)
instead of the pytest ExceptionInfo wrapper.
Tests updated to cover the 4xx/5xx request behavior.
Stop logging str(err) in _request: the login call carries the ArcSight password as a query parameter and a requests exception string usually embeds the full request URL, leaking the credentials into the connector logs. Errors now log only the path, status code and exception type. Also harden check_stream_id so the placeholder is rejected case- insensitively and blank values are caught - the shipped docker-compose.yml used CONNECTOR_LIVE_STREAM_ID=CHANGEME, which the old exact-match guard let through; docker-compose.yml now uses the ChangeMe placeholder consistently (matching config.yml.sample). Correct the _load_config_dict test override return annotations from dict[str, Any] to Self to match the SDK. Adds tests for credential redaction and the stream-id placeholder variants.
check_stream_id() is now called at the start of run(), so a placeholder or blank CONNECTOR_LIVE_STREAM_ID fails fast at startup instead of only when the first stream event arrives. The redundant per-message check in process_message was removed. Covered by a new test_run_aborts_when_stream_id_missing.
"Main dependencies needs to be installed" -> "need" (plural subject).
Address the remaining Copilot review and keep the connector installable: - api_client.py: `_request` now closes the response on the 429/5xx-retry and non-retriable HTTPError paths (the body is never consumed there), and `_post_entries` closes the entry-management response on success. This stops a long-running stream connector from leaving connections checked out of the requests Session pool until garbage collection. - tests/tests_connector/test_settings.py: drop the redundant `is True` on the `isinstance(...)` assertion. - 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 to the same version.
The connector files were committed with CRLF line endings. In a Linux container CRLF breaks the entrypoint shebang (/bin/sh\r) and the cd command, so the container entrypoint can fail at runtime; it is also inconsistent with the rest of the repo and the sibling stream connectors, which use LF. Normalize every file under stream/arcsight/ to LF (content is otherwise unchanged - the diff is line-endings only). Resolves the Copilot review threads flagging CRLF in entrypoint.sh, Dockerfile, docker-compose.yml, src/requirements.txt and tests/test-requirements.txt.
c822b94 to
4a11bf8
Compare
PR review-and-fix + full rebase + pycti
Remaining (non-CI) blocker: |
Proposed changes
This PR adds a new
STREAMconnectorstream/arcsightthat synchronises IOCs from OpenCTI to an ArcSight ESM Active List in real time.LoginServiceto obtain an authentication token (re-issued automatically on expiry).ActiveListService/addEntries); on delete: removes it (ActiveListService/deleteEntries).connectors-sdksettings pattern, with unit tests and connector metadata (manifest, config schema, configuration documentation).Related issues
Closes #6717
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:
/bin/sh\r) and thecdcommand, so the container entrypoint can fail at runtime; it is also inconsistent with the sibling stream connectors. The wholestream/arcsight/tree is now normalized to LF (content otherwise unchanged - the diff is line-endings only)._requestnever logsstr(err). Arequestsexception string typically embeds the full request URL, and the ESMLoginServicelogin call carries the ArcSight password as a query parameter, sostr(err)could leak credentials into the logs. Errors now log only the requestpath, thestatus_codeand the exception type.connector.py:run()validates the live stream id first, so a placeholder/blankCONNECTOR_LIVE_STREAM_IDfails fast at startup instead of only when the first event arrives; the redundant per-message check was removed.connector.py:process_messagecatches only the expected parse errors (json.JSONDecodeError/KeyError/TypeError) and chains viaraise ... from err.api_client.py:_requestretries only on connection/timeout errors, 429 and 5xx; other 4xx (401/403/404) fail fast, which also lets_post_entriestrigger an immediate token re-issue on a 401 instead of after three backoff sleeps.api_client.py: HTTP responses are closed on the 429/5xx-retry and non-retriable HTTPError paths, and_post_entriescloses the entry-management response on success, so a long-running stream connector does not keep connections checked out of the requests Session pool.connector.py/docker-compose.yml:check_stream_id()rejects the placeholder case-insensitively and rejects empty/whitespace-only values, and the shippeddocker-compose.ymluses theChangeMeplaceholder consistently.src/requirements.txt/README.md/__metadata__/connector_manifest.json: the pinnedpyctiis7.260615.0, the latest released version, matching bothconnectors-sdk@masterand thepyctishipped byopencti@master; the README minimum and manifestsupport_versionare aligned to the same version.Decisions (Copilot comment intentionally not applied)
apk update && apk upgradeline is kept: it is byte-for-byte identical tostream/sentinelone-intel/Dockerfileand the other stream connectors (infoblox-threat-defense,harfanglab-intel,sumologic-intel); build-time deps are pruned afterwards withapk del, and the upgrade intentionally pulls Alpine base-image security patches. Keeping it avoids a one-off variant inconsistent with the sibling connectors.Tests
Unit tests cover the settings model (validation, defaults), the connector (fail-fast startup on bad stream id, indicator add/remove dispatch, payload parsing), and the ArcSight client (token issue/re-issue on 401, add/delete entries, STIX value extraction incl. case-insensitive hash keys, 4xx fail-fast vs 429/5xx retry, and log redaction of credentials). 41 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 (6 resolved). The branch has been rebased onto the latestmasteras a clean linear history of signed commits (the previous de-stale merge commit has been dropped), so it merges without a merge commit and the localconnectors-sdkused byrun_test.shresolves the samepycti==7.260615.0consistently.mergeStateStatusis BLOCKED only becausereviewDecisionisREVIEW_REQUIRED- the PR needs one approving review from a maintainer other than me (as the author I cannot self-approve).