fix(recorded-future): include context_entities so their threat actors aren't dropped (#6943) - #7706
Open
Hugo Dupras (jabesq) wants to merge 1 commit into
Open
Hugo Dupras (jabesq) wants to merge 1 commit into
Hugo Dupras (jabesq) wants to merge 1 commit into
Conversation
|
The following packages appear to be unused:
|
|
🔴 Connector Linter errors detected
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates Recorded Future Analyst Note ingestion to retain contextual entities and deduplicate shared entities.
Changes:
- Processes
note_entitiesandcontext_entitiesthrough shared conversion logic. - Deduplicates merged entities by Recorded Future ID.
- Adds regression tests for contextual entities and duplicates.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Summary |
|---|---|
external-import/recorded-future/src/rflib/rf_to_stix2.py |
Includes and deduplicates both entity arrays. |
external-import/recorded-future/tests/tests_connector/test_rf_to_stix2.py |
Adds regression coverage, but the deduplication assertion does not prove both arrays were processed or that only one STIX entity was produced. |
Suppressed comments (1)
external-import/recorded-future/tests/tests_connector/test_rf_to_stix2.py:404
- This assertion does not actually verify the new deduplication path:
StixNote.to_stix_objects()already appliesdict.fromkeyswhen buildingreport.object_refs, so it remains1even if the context entity is processed a second time (and it also passes with the old implementation that ignorescontext_entities). Please adjust the fixture/assertions to prove both arrays were processed and that only one STIX entity object was produced, for example by adding another context-only entity and checking the resulting object count or by spying on the conversion call.
assert len(report.object_refs) == 1
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Proposed changes
context_entitiesalongsidenote_entitiesinStixNote.from_json()(external-import/recorded-future/src/rflib/rf_to_stix2.py). Both arrays now flow through the exact same per-entity conversion logic (type mapping,self.taslookup,person_to_ta,ta_to_intrusion_set, risk-score/threshold handling), so an entity that Recorded Future only tags as contextual is no longer silently dropped.id, so an entity present in both arrays yields a single STIX object and a singleobject_refsentry on the report.tests/tests_connector/test_rf_to_stix2.py:test_analyst_note_includes_context_entities— a Threat Actor present only incontext_entitiesis converted and referenced by the report.test_analyst_note_deduplicates_entities_shared_between_both_arrays— an entity id present in both arrays is referenced only once.Related issues
Checklist
Further comments
Root cause — Recorded Future Analyst Notes carry two entity arrays on
attributes:note_entities(the primary entities the note is directly about) andcontext_entities(related/referenced entities discussed in the note's narrative, which can include threat actors, but are not tagged as primary).StixNote.from_json()only ever iteratedattr.get("note_entities", []). When Recorded Future classified a threat actor named in a report's narrative as a context entity rather than a primary one, the report was still created referencing that actor in its text, but no corresponding STIX Threat Actor / Intrusion Set object (or relationship to it) ever reached the bundle sent to OpenCTI — leaving an incomplete knowledge graph for that report.Validation — the full connector suite passes (84 tests, Python 3.12), along with
isort,black,flake8 --ignore=E,Wand the custom STIX deterministic-ID pylint plugin (nono_generated_id_stixfindings). A revert-check was performed: reverting only thefrom_json()loop change while keeping the new tests makestest_analyst_note_includes_context_entitiesfail withassert 1 == 2(the context-only Threat Actor missing fromreport.object_refs), confirming the regression test genuinely catches the bug.Attribution — this re-submits the work from #7466 (closed) authored by agu2347; the original commit authorship is preserved.