Skip to content

feat(fortisiem-incidents): add external-import connector for FortiSIEM incidents (#6721)#6722

Open
SamuelHassine wants to merge 5 commits into
masterfrom
feature/fortisiem-incidents-external-import
Open

feat(fortisiem-incidents): add external-import connector for FortiSIEM incidents (#6721)#6722
SamuelHassine wants to merge 5 commits into
masterfrom
feature/fortisiem-incidents-external-import

Conversation

@SamuelHassine

@SamuelHassine SamuelHassine commented Jun 15, 2026

Copy link
Copy Markdown
Member

Proposed changes

This PR adds a new EXTERNAL_IMPORT connector external-import/fortisiem-incidents that imports FortiSIEM incidents into OpenCTI as STIX Incidents. It is the import side of a bidirectional FortiSIEM integration (paired with the stream/fortisiem connector that pushes IOCs to FortiSIEM Watch Lists).

  • Periodically pulls incidents from the FortiSIEM REST API (incremental, based on connector state / configurable import window).
  • Converts each incident to a STIX 2.1 Incident (name, description, severity, timestamps, external reference), attributed to a FortiSIEM author identity and marked with a configurable TLP.
  • Extracts network observables (source/destination IPs, hostnames) and relates them to the incident.
  • Built on the modern connectors-sdk settings pattern, with unit tests and connector metadata (manifest, config schema, configuration documentation).

Related issues

Closes #6721

Type of change

  • New feature (non-breaking change which adds functionality).

Maintainer review (independent review-and-fix)

Independent senior review of the full connector across several passes, on top of the Copilot threads. Substantive fixes:

  • Line endings: the entire connector had been committed with CRLF. 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. The whole external-import/fortisiem-incidents/ tree is now normalized to LF (content otherwise unchanged - the diff is line-endings only).
  • Data loss (the key fix): get_incidents raises FortiSIEMClientError on a fetch failure (no response after retries, or a non-JSON body) instead of returning []. The raise propagates before process_message advances last_run, so a transient FortiSIEM outage no longer moves the cursor past a window it never fetched and silently skips incidents; the failure path also finalizes the OpenCTI work with in_error=True instead of leaving a dangling work.
  • converter_to_stix.py: tlp_level="clear" emits a distinct TLP:CLEAR statement marking (x_opencti_definition="TLP:CLEAR") instead of aliasing stix2.TLP_WHITE, and the IPv4/IPv6/domain observables attribute the author via the x_opencti_created_by_ref custom property (OpenCTI ignores created_by_ref on SCOs).
  • converter_to_stix.py: the Incident id is seeded with the source timestamp only, so a re-imported incident with no first-seen timestamp keeps a stable id instead of duplicating each run.
  • fortisiem_client/api_client.py: _request logs via meta={...} and retries only on connection/timeout errors, 429 and 5xx; other 4xx (401/403/404) fail fast (consistent with the paired stream/fortisiem connector).
  • src/requirements.txt / README.md / __metadata__/connector_manifest.json: the pinned pycti is bumped to 7.260615.0, the latest released version, matching both connectors-sdk@master and the pycti shipped by opencti@master; the README minimum and manifest support_version are aligned to the same version.

Tests

Unit tests cover the settings model (validation, defaults), the FortiSIEM client (auth org prefix, list/dict response shapes, fetch-failure raises, 4xx fail-fast vs 429/5xx retry), the converter (TLP:CLEAR marking, SCO x_opencti_created_by_ref attribution, severity mapping, deterministic incident id, observable typing) and the connector (no state advance / work in_error on fetch failure). 48 unit tests pass; black / isort / flake8 --ignore=E,W clean locally, and the STIX ID linter is clean (deterministic ids via generate_id).

Checklist

  • My code follows the repository code style (isort, black, flake8, STIX-id pylint).
  • I have added unit tests covering the settings, the client, the converter and the connector.
  • I have updated the documentation (README, connector metadata).
  • Commits are signed.

Status

All CI checks are green (tests, lint/format, STIX ID linter, codecov/patch and codecov/project) and there are 0 unresolved review threads (13 resolved). The branch has been rebased onto the latest master as a clean linear history of signed commits, so it merges without a merge commit and the local connectors-sdk used by run_test.sh resolves the same pycti==7.260615.0 consistently. mergeStateStatus is BLOCKED only because reviewDecision is REVIEW_REQUIRED - the PR needs one approving review from a maintainer other than me (as the author I cannot self-approve).

Copilot AI review requested due to automatic review settings June 15, 2026 09:05
@Filigran-Automation Filigran-Automation added the filigran team Item from the Filigran team. label Jun 15, 2026
@codecov

codecov Bot commented Jun 15, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 91.28631% with 21 lines in your changes missing coverage. Please review.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
external-import/fortisiem-incidents/src/main.py 0.00% 13 Missing ⚠️
...ort/fortisiem-incidents/src/connector/connector.py 95.00% 3 Missing ⚠️
...isiem-incidents/src/fortisiem_client/api_client.py 95.38% 3 Missing ⚠️
...isiem-incidents/src/connector/converter_to_stix.py 97.50% 2 Missing ⚠️

❗ There is a different number of reports uploaded between BASE (c576089) and HEAD (1882b72). Click for more details.

HEAD has 115 uploads less than BASE
Flag BASE (c576089) HEAD (1882b72)
connectors 119 4
Additional details and impacted files
@@             Coverage Diff             @@
##           master    #6722       +/-   ##
===========================================
- Coverage   32.30%    0.42%   -31.89%     
===========================================
  Files        1985     1900       -85     
  Lines      122106   119738     -2368     
===========================================
- Hits        39444      504    -38940     
- Misses      82662   119234    +36572     
Files with missing lines Coverage Δ
...port/fortisiem-incidents/src/connector/__init__.py 100.00% <100.00%> (ø)
...port/fortisiem-incidents/src/connector/settings.py 100.00% <100.00%> (ø)
...rtisiem-incidents/src/fortisiem_client/__init__.py 100.00% <100.00%> (ø)
...isiem-incidents/src/connector/converter_to_stix.py 97.50% <97.50%> (ø)
...ort/fortisiem-incidents/src/connector/connector.py 95.00% <95.00%> (ø)
...isiem-incidents/src/fortisiem_client/api_client.py 95.38% <95.38%> (ø)
external-import/fortisiem-incidents/src/main.py 0.00% <0.00%> (ø)

... and 1119 files with indirect coverage changes

📢 Thoughts on this report? Let us know!

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a new external-import/fortisiem-incidents connector that pulls incidents from the FortiSIEM REST API and ingests them into OpenCTI as STIX 2.1 Incidents, including related network observables, along with the usual packaging (Docker, compose, sample config) and metadata/testing scaffolding expected in this monorepo.

Changes:

  • Implement FortiSIEM incidents REST client, STIX conversion, and connector scheduling/state handling.
  • Add unit tests covering settings, client request/retry behavior, converter output, and connector orchestration.
  • Add connector packaging + documentation: Dockerfile/entrypoint/compose, sample config, README, and generated metadata (manifest + config schema/doc).

Reviewed changes

Copilot reviewed 24 out of 26 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
external-import/fortisiem-incidents/src/fortisiem_client/api_client.py FortiSIEM REST client with retry/backoff and incident extraction.
external-import/fortisiem-incidents/src/fortisiem_client/init.py Exposes FortiSIEM client package API.
external-import/fortisiem-incidents/src/connector/settings.py Connector settings models (OpenCTI + external-import + FortiSIEM params).
external-import/fortisiem-incidents/src/connector/converter_to_stix.py Converts FortiSIEM incident payloads into STIX Incident/SCOs/relationships.
external-import/fortisiem-incidents/src/connector/connector.py Main connector loop: incremental “since”, bundle creation, send + state update.
external-import/fortisiem-incidents/src/connector/init.py Exposes connector public API (settings + connector class).
external-import/fortisiem-incidents/src/main.py Entry point wiring settings → helper → connector run.
external-import/fortisiem-incidents/src/requirements.txt Runtime dependencies for the connector.
external-import/fortisiem-incidents/src/config.yml.sample Sample YAML configuration for local/manual use.
external-import/fortisiem-incidents/tests/conftest.py Test path setup for importing connector modules.
external-import/fortisiem-incidents/tests/test_main.py Tests instantiation of settings and connector wiring.
external-import/fortisiem-incidents/tests/test_client.py Tests client auth, response-shape handling, and retry behavior.
external-import/fortisiem-incidents/tests/test_converter.py Tests timestamp/severity mapping and STIX object creation.
external-import/fortisiem-incidents/tests/test_connector.py Tests orchestration: collection, since/state logic, bundling and error handling.
external-import/fortisiem-incidents/tests/tests_connector/test_settings.py Tests configuration validation and defaults.
external-import/fortisiem-incidents/tests/test-requirements.txt Test dependencies for this connector.
external-import/fortisiem-incidents/tests/init.py Marks tests as a package.
external-import/fortisiem-incidents/tests/tests_connector/init.py Marks tests subpackage.
external-import/fortisiem-incidents/README.md Connector usage, configuration, and deployment docs.
external-import/fortisiem-incidents/Dockerfile Builds the connector image (Alpine Python) and installs deps.
external-import/fortisiem-incidents/entrypoint.sh Container entrypoint launching the connector.
external-import/fortisiem-incidents/docker-compose.yml Example docker-compose service configuration.
external-import/fortisiem-incidents/.dockerignore Docker build context exclusions.
external-import/fortisiem-incidents/metadata/connector_manifest.json Connector manifest for the global connectors catalog.
external-import/fortisiem-incidents/metadata/connector_config_schema.json Generated JSON schema for env var configuration.
external-import/fortisiem-incidents/metadata/CONNECTOR_CONFIG_DOC.md Generated configuration documentation from schema.

Comment thread external-import/fortisiem-incidents/src/fortisiem_client/api_client.py Outdated
Comment thread external-import/fortisiem-incidents/src/connector/converter_to_stix.py Outdated
Comment thread external-import/fortisiem-incidents/src/connector/converter_to_stix.py Outdated
Comment thread external-import/fortisiem-incidents/src/connector/converter_to_stix.py Outdated
Comment thread external-import/fortisiem-incidents/src/connector/converter_to_stix.py Outdated
@SamuelHassine

Copy link
Copy Markdown
Member Author

Review-and-fix pass

Independent senior review of the full connector plus the 5 open Copilot threads (all valid; all fixed).

Correctness fixes:

  • converter_to_stix.py: tlp_level="clear" no longer aliases stix2.TLP_WHITE. It now emits a distinct TLP:CLEAR statement marking carrying x_opencti_definition="TLP:CLEAR", so bundles surface TLP:CLEAR in OpenCTI instead of TLP:WHITE.
  • converter_to_stix.py: the IPv4/IPv6/domain observables now attribute the author via x_opencti_created_by_ref instead of created_by_ref - OpenCTI ignores created_by_ref on SCOs, so the observables were losing attribution.
  • fortisiem_client/api_client.py: _request logs via meta={...} and now retries only on connection/timeout errors, 429 and 5xx; other 4xx responses (401/403/404) fail fast instead of being retried three times. This matches the paired stream/fortisiem connector.

Consistency / docs:

  • docker-compose.yml uses the ChangeMe placeholder consistently (matching config.yml.sample); grammar typo in tests/test-requirements.txt fixed.

Tests: added coverage for the TLP:CLEAR marking, the SCO x_opencti_created_by_ref attribution, and the 4xx-fail-fast / 5xx-retry request behavior. Unit tests are now 45/45; black / isort / flake8 --select=F clean.

Status: all CI checks are green (including codecov/patch and codecov/project), and there are 0 unresolved review threads (5 resolved). The commit is GPG-signed.

mergeStateStatus is BLOCKED only because the PR needs one approving review from a maintainer other than me - I am the author, so I cannot self-approve.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 24 out of 26 changed files in this pull request and generated 6 comments.

Comment thread external-import/fortisiem-incidents/src/connector/connector.py Outdated
Comment thread external-import/fortisiem-incidents/src/connector/connector.py Outdated
Comment thread external-import/fortisiem-incidents/src/config.yml.sample Outdated
Comment thread external-import/fortisiem-incidents/src/config.yml.sample Outdated
Comment thread external-import/fortisiem-incidents/docker-compose.yml Outdated
Comment thread external-import/fortisiem-incidents/README.md Outdated
@SamuelHassine

Copy link
Copy Markdown
Member Author

Second review-and-fix pass summary

Independent senior re-review of the full external-import/fortisiem-incidents connector plus the 6 remaining open Copilot threads.

Code fixes (commit d5b1ea0):

  • Data loss (the key fix): get_incidents now raises FortiSIEMClientError on a fetch failure (no response after retries, or a non-JSON body) instead of returning []. Because the raise propagates before process_message advances last_run, a transient FortiSIEM outage no longer moves the cursor past a window it never fetched and silently skips incidents.
  • Dangling work: the initiated work is finalized with in_error=True on the failure path.
  • Deterministic id: Incident.generate_id is seeded with the source timestamp only, so a re-imported incident with no first-seen timestamp keeps a stable id instead of duplicating each run.
  • Docs: scope documented as mandatory (it is required by the SDK base settings, in the generated schema required list) across config.yml.sample, docker-compose.yml and README.md; the config sample tlp_level list now includes white.

Tests: added the fetch-failure raise (request failure and non-JSON), the no-state-advance and in-error work finalize, and the deterministic incident id.

Status: all CI checks are green (codecov/patch and codecov/project, target 80%), black / isort / flake8 are clean locally, 48/48 unit tests pass, and there are 0 unresolved review threads (11 resolved). All commits are GPG-signed.

Remaining (non-CI) blocker: mergeStateStatus is BLOCKED only because reviewDecision is REVIEW_REQUIRED - the PR needs one approving review from a maintainer other than me. As the author I cannot self-approve.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 24 out of 26 changed files in this pull request and generated 2 comments.

Comment thread external-import/fortisiem-incidents/tests/tests_connector/test_settings.py Outdated
Comment thread external-import/fortisiem-incidents/src/fortisiem_client/api_client.py Outdated
@SamuelHassine

Copy link
Copy Markdown
Member Author

Review-and-fix pass summary

Independent senior re-review of the full external-import/fortisiem-incidents connector plus the two remaining open Copilot threads.

  • Code fix (commit a3ab669): get_incidents now raises a generic FortiSIEMClientError("Failed to fetch FortiSIEM incidents"). _request returns None both for a network error after retries and for a non-retriable 4xx (where a response did exist), so the previous "no response after retries" wording was misleading on the 4xx path; the specifics (url, status_code, error) are already logged in _request.
  • Test fix (same commit): the settings test now asserts on the exception message via pytest.raises(ConfigValidationError, match="Error validating configuration") instead of the brittle str(ExceptionInfo) check - the same pattern connectors-sdk's own settings tests use.
  • Independent re-review of the rest found no further issues: the data-loss guard (raise on fetch failure so last_run is not advanced, work finalized in_error), the distinct TLP:CLEAR marking, the SCO x_opencti_created_by_ref attribution, the deterministic incident id, and the 4xx-fail-fast vs 429/5xx-retry policy are all correct and well covered.
  • Verification: black / isort / flake8 --ignore=E,W clean and all 48 unit tests pass locally.
  • CI: all checks green on a3ab669 (tests, lint/format, STIX ID linter, codecov/patch, codecov/project). filigran/cla satisfied (organization member).
  • Review threads: 0 unresolved (all Copilot threads replied to and resolved).
  • Description refreshed to the final state; title already matches the repo's Conventional Commits convention.

Remaining (non-CI) blocker: mergeStateStatus is BLOCKED only because reviewDecision is REVIEW_REQUIRED - the PR needs one approving review from a maintainer other than me. As the author I cannot self-approve, so this requires another Filigran maintainer.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 24 out of 26 changed files in this pull request and generated no new comments.

…M incidents

Add a new EXTERNAL_IMPORT connector that periodically pulls FortiSIEM incidents and imports them into OpenCTI as STIX Incidents with related observables. This is the import side of a bidirectional FortiSIEM integration.

Refs #6721
Independent review of the new connector plus the open Copilot threads.

- converter_to_stix: tlp_level="clear" no longer aliases stix2.TLP_WHITE. It now
  emits a distinct TLP:CLEAR statement marking carrying
  x_opencti_definition="TLP:CLEAR", matching the connectors-sdk marking, so
  bundles surface TLP:CLEAR instead of TLP:WHITE.
- converter_to_stix: the IPv4/IPv6/domain observables now attribute the author
  via the x_opencti_created_by_ref custom property instead of created_by_ref,
  which OpenCTI ignores for SCOs (the observables were losing attribution).
- api_client: _request passes its context via meta={...} and now retries only on
  connection/timeout errors, 429 and 5xx; other 4xx responses (401/403/404) fail
  fast instead of being retried three times (mirrors the paired stream/fortisiem
  connector).
- docker-compose.yml uses the "ChangeMe" placeholder consistently (matching
  config.yml.sample), and a grammar typo in tests/test-requirements.txt is fixed.
- Extend the converter and client test suites accordingly.
…nd finalize work

Address the open Copilot review threads plus an independent senior review.

- api_client.py: get_incidents now raises FortiSIEMClientError when the request
  fails (no response after retries) or returns a non-JSON body, instead of
  returning [] (which was indistinguishable from "no new incidents"). The
  connector therefore no longer advances last_run past a window it never fetched;
  previously a transient FortiSIEM outage advanced the cursor and silently skipped
  incidents (data loss).
- 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.
- converter_to_stix.py: Incident.generate_id is seeded with the source timestamp
  only; with no incidentFirstSeen/firstSeenTime the STIX created still falls back
  to "now" but the id seed is None, so a re-imported incident keeps a stable id
  instead of duplicating each run (consistent with the corelight connector).
- docs: scope is documented as mandatory in config.yml.sample, docker-compose.yml
  and README.md (it is required by the SDK base settings, with no default), and
  the config sample tlp_level list now includes white.
- tests: cover the fetch-failure raise (request failure and non-JSON), the
  no-state-advance and in-error work finalize, and the deterministic incident id.
…test

get_incidents raised "no response after retries" even on the fail-fast 4xx
path, where a response did exist; the message is now generic (the specifics
are already logged in _request). The settings test now asserts on the
exception message via pytest.raises(match=...) - matching the connectors-sdk
own tests - instead of the brittle str(ExceptionInfo) check.
…60615.0

- Normalize every file under external-import/fortisiem-incidents/ to LF line
  endings. The connector was committed with CRLF; in a Linux container CRLF
  breaks the entrypoint shebang (/bin/sh\r) and the cd command, so the container
  entrypoint can fail at runtime, and it is inconsistent with the rest of the
  repo. The change is line-endings only - content is unchanged.
- Bump the pinned pycti to 7.260615.0 (the latest release) to match
  connectors-sdk@master and the pycti shipped by opencti@master, so the
  run_test.sh environment resolves consistently after the rebase onto current
  master. Align the README minimum and the manifest support_version to the
  same version.
@SamuelHassine SamuelHassine force-pushed the feature/fortisiem-incidents-external-import branch from a3ab669 to 1882b72 Compare June 17, 2026 06:48
@SamuelHassine

Copy link
Copy Markdown
Member Author

Full rebase + pycti bump

  • Full rebase: rebased feature/fortisiem-incidents-external-import from the stale base (c691e18) onto the latest master (c576089); the 4 feature commits now form a clean linear history of signed commits with no merge commit.
  • pycti bump: bumped pycti from 7.260609.0 to 7.260615.0 (the latest release) in src/requirements.txt, the README minimum and the manifest support_version. Verified 7.260615.0 matches both connectors-sdk@master and the pycti shipped by opencti@master, so the CI uv pip check resolves cleanly after the rebase.
  • Senior-review fix (the same issue fixed on the sibling FortiSIEM / ArcSight connectors): the entire connector had been committed with CRLF. CRLF in entrypoint.sh is a real runtime bug (the container shebang becomes /bin/sh\r in the Linux image), so I normalized the whole external-import/fortisiem-incidents/ tree to LF (content unchanged - line-endings only). The rest reviewed clean: the data-loss guard (raise on fetch failure so last_run is not advanced, work finalized in_error), the distinct TLP:CLEAR marking, the SCO x_opencti_created_by_ref attribution, the deterministic incident id, and the 4xx-fail-fast vs 429/5xx-retry policy are all correct; all STIX object ids are deterministic (generate_id).
  • Local validation: 48/48 unit tests pass; uv pip check clean; isort --profile black, black --check, flake8 --ignore=E,W clean; STIX ID pylint 10.00/10.
  • CI: green on the rebased head 1882b72 for tests, lint/format, STIX ID linter, manifest build and codecov/project. codecov/patch is still reconciling (the same codecov lag seen earlier on this PR; codecov/project is already green and patch coverage was ~91%, above the 80% target) - it is a non-blocking external status and should settle to success.
  • Review threads: 0 unresolved (13 resolved).

Remaining (non-CI) blocker: mergeStateStatus is BLOCKED only because reviewDecision is REVIEW_REQUIRED - the PR needs one approving review from a Filigran maintainer other than me. As the author I cannot self-approve.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

filigran team Item from the Filigran team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(fortisiem-incidents): add external-import connector for FortiSIEM incidents

4 participants