[Bug] GenericLoader incorrectly flags name collisions across different resource types

[pberba]

Describe the Bug

The names of the generic loader are checked for uniqueness. This can cause collisions for actions and exceptions` that share the same name.

https://github.com/elastic/detection-rules/blob/66a0b6b97c47957e5019d681943f4ff8ed3470ac/detection_rules/generic_loader.py#L110-L111

To Reproduce

This can happen when there is:

1. There is a shared list with list_name X
2. Action that points to a rule_name also X

Because TOMLAction.name is based on rule_name and TOMLException.name is based on list_name, the two resources' names will collide.

Expected Behavior

Collisions should only occur between actions only or exceptions only and not with each other

Screenshots

No response

Desktop - OS

None

Desktop - Version

No response

Additional Context

No response

[eric-forte-elastic]

👋 Hi @pberba, unfortunately the collision on name is by design. The generic loader needs to support the following types: TOMLAction | TOMLActionConnector | TOMLException These types are not all required to have an id field for comparison, but they are required to have a name. If we take TOMLActionConnector for instance, by looking at the object we can see:

class TOMLActionConnector:
    """Object for action connector from TOML file."""

    contents: TOMLActionConnectorContents
    path: Path

    @property
    def name(self) -> str:
        return self.contents.metadata.action_connector_name

Where TOMLActionConnectorContents is defined as:

@dataclass(frozen=True)
class TOMLActionConnectorContents(MarshmallowDataclassMixin):
    """Object for action connector from TOML file."""

    metadata: ActionConnectorMeta
    action_connectors: list[ActionConnector]

In this case, the TOMLActionConnector has a name property, but no id property. Additionally, the same TOML Action Connector file can have a list of action_connectors (e.g. action_connectors: list[ActionConnector]) which is where the ids for the connectors come from. In Kibana it is valid to have two identical by content Action Connector Lists (or identical TOMLActionConnectors) that are linking the same list of different ActionConnectors to different sets of rules. Additionally, we cannot rely on the id field in Kibana for these objects as IDs for action connectors need to be unique across spaces. E.g. if a given ActionContectorList Toml file was imported to both a production and dev space then the id would be regenerated for one of those spaces and would thereby change the contents and reference if we used the ID (see https://github.com/elastic/kibana/pull/223454 for reference).

Given this, we need to use name as the underlying identifier, these should be thought of as uuids for our purposes here.

As far as the request relating to TOMLException and TOMLAction we would then need to implement separate loaders for the different collection types. But since we are using name as a uuid in this case, we may not want them to collide even across different objects when being packaged into a single json. This is not so much as a bug, rather more of a design decision. Granted this could be revisited and changed, but for the moment this collision is not incorrect.