From daf0d0ad92c0e3f8e908eef6dda0facf839442d5 Mon Sep 17 00:00:00 2001 From: Jared Ondricek Date: Wed, 30 Sep 2026 09:48:41 -0500 Subject: [PATCH 1/2] feat: add tests for handling missing ATT&CK object URLs in changelog generation --- mitreattack/diffStix/changelog_helper.py | 94 +++++++++++---- .../formatting/test_missing_attack_urls.py | 113 ++++++++++++++++++ 2 files changed, 184 insertions(+), 23 deletions(-) create mode 100644 tests/changelog/formatting/test_missing_attack_urls.py diff --git a/mitreattack/diffStix/changelog_helper.py b/mitreattack/diffStix/changelog_helper.py index fe88b70..d16e656 100644 --- a/mitreattack/diffStix/changelog_helper.py +++ b/mitreattack/diffStix/changelog_helper.py @@ -11,6 +11,7 @@ from dataclasses import dataclass from pathlib import Path from typing import Any, Dict, List, Optional +from urllib.parse import urlsplit import markdown import requests @@ -1070,8 +1071,10 @@ def placard(self, stix_object: dict, section: str, domain: str) -> str: ) parent_name = parent_object.get("name", "ERROR NO PARENT") relative_url = get_relative_url_from_stix(stix_object=revoker) - revoker_link = f"{self.site_prefix}/{relative_url}" - placard_string = f"{revoked_name} (revoked by {parent_name}: [{revoker['name']}]({revoker_link}))" + revoker_name = ( + f"[{revoker['name']}]({self.site_prefix}/{relative_url})" if relative_url else revoker["name"] + ) + placard_string = f"{revoked_name} (revoked by {parent_name}: {revoker_name})" elif revoker["type"] == "x-mitre-data-component": parent_object = self.get_parent_stix_object( @@ -1079,17 +1082,21 @@ def placard(self, stix_object: dict, section: str, domain: str) -> str: ) if parent_object: parent_name = parent_object.get("name", "ERROR NO PARENT") - relative_url = get_relative_data_component_url(datasource=parent_object, datacomponent=stix_object) - revoker_link = f"{self.site_prefix}/{relative_url}" - placard_string = f"{revoked_name} (revoked by {parent_name}: [{revoker['name']}]({revoker_link}))" + relative_url = get_relative_data_component_url(datasource=parent_object, datacomponent=revoker) + revoker_name = ( + f"[{revoker['name']}]({self.site_prefix}/{relative_url})" if relative_url else revoker["name"] + ) + placard_string = f"{revoked_name} (revoked by {parent_name}: {revoker_name})" else: # No parent datasource available — fall back to a plain-text representation. placard_string = f"{revoked_name} (revoked by {revoker['name']})" else: relative_url = get_relative_url_from_stix(stix_object=revoker) - revoker_link = f"{self.site_prefix}/{relative_url}" - placard_string = f"{revoked_name} (revoked by [{revoker['name']}]({revoker_link}))" + revoker_name = ( + f"[{revoker['name']}]({self.site_prefix}/{relative_url})" if relative_url else revoker["name"] + ) + placard_string = f"{revoked_name} (revoked by {revoker_name})" else: if stix_object["type"] == "x-mitre-data-component": @@ -1098,14 +1105,22 @@ def placard(self, stix_object: dict, section: str, domain: str) -> str: ) if parent_object: relative_url = get_relative_data_component_url(datasource=parent_object, datacomponent=stix_object) - placard_string = f"[{stix_object['name']}]({self.site_prefix}/{relative_url})" + placard_string = ( + f"[{stix_object['name']}]({self.site_prefix}/{relative_url})" + if relative_url + else stix_object["name"] + ) else: # No parent datasource available — display datacomponent name as plain text. placard_string = stix_object["name"] else: relative_url = get_relative_url_from_stix(stix_object=stix_object) - placard_string = f"[{stix_object['name']}]({self.site_prefix}/{relative_url})" + placard_string = ( + f"[{stix_object['name']}]({self.site_prefix}/{relative_url})" + if relative_url + else stix_object["name"] + ) placard_string = self.prefix_with_parent_name( stix_object=stix_object, @@ -1704,32 +1719,65 @@ def is_patch_change(old_stix_obj: dict, new_stix_obj: dict) -> bool: def get_relative_url_from_stix(stix_object: dict) -> Optional[str]: - """Parse the website url from a stix object. + """Get an ATT&CK website path from the object's first external reference. Parameters ---------- stix_object : dict - An ATT&CK STIX Domain Object (SDO). + ATT&CK STIX object. Returns ------- Optional[str] - The relative URL for the ATT&CK object. + Relative website path, or None if the first reference has no usable ATT&CK URL. """ - is_subtechnique = stix_object["type"] == "attack-pattern" and stix_object.get("x_mitre_is_subtechnique") - - if stix_object.get("external_references"): - url = stix_object["external_references"][0]["url"] - split_url = url.split("/") - splitfrom = -3 if is_subtechnique else -2 - link = "/".join(split_url[splitfrom:]) - return link + is_subtechnique = stix_object.get("type") == "attack-pattern" and stix_object.get("x_mitre_is_subtechnique") + + attack_sources = {"mitre-attack", "mitre-mobile-attack", "mitre-ics-attack"} + references = stix_object.get("external_references") or [] + reference = references[0] if references else {} + attack_reference = reference.get("source_name") in attack_sources + url = reference.get("url") if attack_reference else None + if isinstance(url, str): + try: + parsed = urlsplit(url.strip()) + except ValueError: + parsed = None + if parsed and parsed.scheme in {"http", "https"} and parsed.netloc.lower() == "attack.mitre.org": + path_parts = parsed.path.strip("/").split("/") + required_parts = 3 if is_subtechnique else 2 + if len(path_parts) >= required_parts and all(path_parts[-required_parts:]): + return "/".join(path_parts[-required_parts:]) + + stix_id = stix_object.get("id", "unknown") + attack_id = (reference.get("external_id") if attack_reference else None) or "unknown" + name = stix_object.get("name", "unknown") + logger.warning( + f"No usable ATT&CK URL for STIX ID {stix_id}, ATT&CK ID {attack_id}, name {name}; " + "add a valid attack.mitre.org URL to its ATT&CK external reference" + ) return None -def get_relative_data_component_url(datasource: dict, datacomponent: dict) -> str: - """Create url of data component with parent data source.""" - return f"{get_relative_url_from_stix(stix_object=datasource)}/#{'%20'.join(datacomponent['name'].split(' '))}" +def get_relative_data_component_url(datasource: dict, datacomponent: dict) -> Optional[str]: + """Get a data component's website path using its data source URL. + + Parameters + ---------- + datasource : dict + Parent ATT&CK data source. + datacomponent : dict + Data component whose name becomes the URL anchor. + + Returns + ------- + Optional[str] + Relative path and component anchor, or None if the data source has no usable ATT&CK URL. + """ + datasource_url = get_relative_url_from_stix(stix_object=datasource) + if datasource_url is None: + return None + return f"{datasource_url}/#{'%20'.join(datacomponent['name'].split(' '))}" def deep_copy_stix(stix_objects: List[dict]) -> List[dict]: diff --git a/tests/changelog/formatting/test_missing_attack_urls.py b/tests/changelog/formatting/test_missing_attack_urls.py new file mode 100644 index 0000000..6c2d494 --- /dev/null +++ b/tests/changelog/formatting/test_missing_attack_urls.py @@ -0,0 +1,113 @@ +"""Markdown fallbacks for ATT&CK objects without website URLs.""" + +from unittest.mock import patch + +import pytest + +from mitreattack.diffStix.changelog_helper import get_relative_data_component_url, get_relative_url_from_stix + + +def test_later_attack_reference_does_not_supply_url(sample_technique_object): + """Only the first external reference can supply an object URL.""" + sample_technique_object["external_references"][0].pop("url") + sample_technique_object["external_references"].extend( + [ + {"source_name": "citation", "url": "https://example.org/other/T1234"}, + { + "source_name": "mitre-attack", + "external_id": "T1234", + "url": "https://attack.mitre.org/techniques/T1234/", + }, + ] + ) + + assert get_relative_url_from_stix(sample_technique_object) is None + + +def test_citation_first_does_not_use_later_attack_url(sample_technique_object): + """Do not search past a first reference that is a citation.""" + sample_technique_object["external_references"].insert( + 0, {"source_name": "citation", "url": "https://example.org/other/T1234"} + ) + + assert get_relative_url_from_stix(sample_technique_object) is None + + +@pytest.mark.parametrize("url", [None, "", "https://example.org/techniques/T1234", "https://attack.mitre.org/"]) +def test_unusable_attack_url_is_not_linked(sample_technique_object, url): + """Reject missing, unrelated, and incomplete website URLs.""" + sample_technique_object["external_references"][0]["url"] = url + assert get_relative_url_from_stix(sample_technique_object) is None + + +def test_plain_text_placard_when_url_missing(lightweight_diffstix, sample_technique_object): + """Keep a changed object's placard when its website URL is missing.""" + sample_technique_object["external_references"][0].pop("url") + + result = lightweight_diffstix.placard(sample_technique_object, "additions", "enterprise-attack") + + assert result.startswith(sample_technique_object["name"]) + assert "[" not in result + assert "/None" not in result + + section = lightweight_diffstix.get_markdown_section_data( + [{"parent": sample_technique_object, "children": []}], "additions", "enterprise-attack" + ) + assert f"* {sample_technique_object['name']}" in section + + +@pytest.mark.parametrize("subtechnique", [False, True]) +def test_revoker_without_url_is_plain_text( + lightweight_diffstix, sample_technique_object, mock_stix_object_factory, subtechnique +): + """Keep revocation text for both ordinary and subtechnique revokers.""" + revoker = mock_stix_object_factory(name="Replacement Technique", attack_id="T9999") + revoker["external_references"][0].pop("url") + revoker["x_mitre_is_subtechnique"] = subtechnique + sample_technique_object["revoked_by"] = revoker + + result = lightweight_diffstix.placard(sample_technique_object, "revocations", "enterprise-attack") + + assert "revoked by" in result + assert "Replacement Technique" in result + assert "[Replacement Technique]" not in result + assert "/None" not in result + + +def test_data_component_without_parent_url_is_plain_text( + lightweight_diffstix, sample_data_source_object, sample_data_component_object +): + """Keep component text when its parent data source lacks a URL.""" + sample_data_source_object["external_references"][0].pop("url") + sample_data_component_object["x_mitre_data_source_ref"] = sample_data_source_object["id"] + lightweight_diffstix.data["new"]["enterprise-attack"]["attack_objects"]["datasources"] = { + sample_data_source_object["id"]: sample_data_source_object + } + + assert get_relative_data_component_url(sample_data_source_object, sample_data_component_object) is None + result = lightweight_diffstix.placard(sample_data_component_object, "additions", "enterprise-attack") + + assert "Test Data Source: Test Data Component" in result + assert "[Test Data Component]" not in result + assert "/None" not in result + + +def test_revoking_data_component_uses_its_name_in_link( + lightweight_diffstix, sample_technique_object, sample_data_source_object, sample_data_component_object +): + """Link a revoking component by its own anchor and fall back to text.""" + sample_data_component_object["x_mitre_data_source_ref"] = sample_data_source_object["id"] + lightweight_diffstix.data["new"]["enterprise-attack"]["attack_objects"]["datasources"] = { + sample_data_source_object["id"]: sample_data_source_object + } + sample_technique_object["revoked_by"] = sample_data_component_object + + result = lightweight_diffstix.placard(sample_technique_object, "revocations", "enterprise-attack") + + assert "#Test%20Data%20Component" in result + + sample_data_source_object["external_references"][0].pop("url") + result = lightweight_diffstix.placard(sample_technique_object, "revocations", "enterprise-attack") + assert "Test Data Source: Test Data Component" in result + assert "[Test Data Component]" not in result + assert "/None" not in result From 7f0c7a5c4d2f95f254f73d026ccba8e90e9084b4 Mon Sep 17 00:00:00 2001 From: Jared Ondricek Date: Wed, 30 Sep 2026 09:55:01 -0500 Subject: [PATCH 2/2] test: remove unused import from missing URL tests --- tests/changelog/formatting/test_missing_attack_urls.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/changelog/formatting/test_missing_attack_urls.py b/tests/changelog/formatting/test_missing_attack_urls.py index 6c2d494..b4a1691 100644 --- a/tests/changelog/formatting/test_missing_attack_urls.py +++ b/tests/changelog/formatting/test_missing_attack_urls.py @@ -1,7 +1,5 @@ """Markdown fallbacks for ATT&CK objects without website URLs.""" -from unittest.mock import patch - import pytest from mitreattack.diffStix.changelog_helper import get_relative_data_component_url, get_relative_url_from_stix