Skip unallowed common properties provided by ObjectFactory#606
Skip unallowed common properties provided by ObjectFactory#606SYNchroACK wants to merge 1 commit intooasis-open:masterfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes an issue where the ObjectFactory class was incorrectly applying default properties (created, created_by_ref, external_references) to STIX Cyber-observable Objects (SCOs), which do not support these properties according to the STIX specification. The fix adds logic to detect when an observable is being created and removes these incompatible properties before instantiation.
Changes:
- Added property filtering logic in
ObjectFactory.create()to remove SCO-incompatible properties - Detects observables by checking if "observables" is in the class module name
- Removes
created,created_by_ref, andexternal_referencesproperties for observable objects
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| properties = copy.deepcopy(self._defaults) | ||
|
|
||
| # SCOs do not have these common properties provided by the factory | ||
| if "observables" in cls.__module__: |
There was a problem hiding this comment.
The module name checking approach using string matching is fragile and could break if module structure changes. Consider using a more robust approach like checking if the class is a subclass of _Observable. You would need to import _Observable from .base and then use: issubclass(cls, _Observable) instead of "observables" in cls.__module__.
| if "observables" in cls.__module__: | ||
| properties.pop("created", None) | ||
| properties.pop("created_by_ref", None) | ||
| properties.pop("external_references", None) |
There was a problem hiding this comment.
The modified property is also set by the factory's set_default_created() method (see line 61) and should be removed for SCOs since they don't have this property. Add properties.pop("modified", None) to ensure the modified property is also removed for observables.
| properties.pop("external_references", None) | |
| properties.pop("external_references", None) | |
| properties.pop("modified", None) |
| # SCOs do not have these common properties provided by the factory | ||
| if "observables" in cls.__module__: | ||
| properties.pop("created", None) | ||
| properties.pop("created_by_ref", None) | ||
| properties.pop("external_references", None) | ||
|
|
There was a problem hiding this comment.
The new functionality for filtering out properties when creating observables lacks test coverage. Consider adding a test that creates an observable (like DomainName) using an ObjectFactory initialized with created_by_ref or created properties to ensure these properties are correctly filtered out.
Problem: the existing
ObjectFactorywhen initialized with the default values likecreated_by_ref, does not handle properly the creation of SCOs, which does not have these default properties (created,created_by_ref,external_references) accordingly to the standard.Test
snippet.py:Without the changes proposed in this pull request:
With the changes proposed in this pull request: