From e67540c80d0b455c64d2bd16d84dcf91904a7d2e Mon Sep 17 00:00:00 2001 From: Ville Laitila Date: Tue, 11 Aug 2026 16:22:29 +0300 Subject: [PATCH 01/11] SBOM: design for exposing the element's model path Publishes each SBOM's model location as a native CycloneDX 'group' (the parent element's full path) plus a 'softagram:elementPath' property, and adds the repository URL as a vcs externalReference. Additive only: bom-ref, serialNumber, purl and version are untouched. --- .../2026-08-11-sbom-element-path-design.md | 291 ++++++++++++++++++ 1 file changed, 291 insertions(+) create mode 100644 docs/superpowers/specs/2026-08-11-sbom-element-path-design.md diff --git a/docs/superpowers/specs/2026-08-11-sbom-element-path-design.md b/docs/superpowers/specs/2026-08-11-sbom-element-path-design.md new file mode 100644 index 0000000..0b26940 --- /dev/null +++ b/docs/superpowers/specs/2026-08-11-sbom-element-path-design.md @@ -0,0 +1,291 @@ +# SBOM: expose the element's model path + +Design for a change to `src/sgraph/converters/sbom_cyclonedx_generator.py`. + +Baseline: sgraph **1.9.0** (`upstream/main` @ `fb65e0d`), 237 tests green. + +--- + +## 1. The problem + +At `--level 3` the per-repository SBOM identifies the repository by name only: + +```json +{ "bom-ref": "repoa", + "name": "repoA", + "type": "application", "version": "", "purl": "", "externalReferences": [] } +``` + +Nothing says the element lives under `/OrgName/GroupA`. The bulk export is the only endpoint +usable at scale, so a consumer that needs the parent group has to download the whole model XML +— hundreds of megabytes on a large estate — purely to rebuild a tree the SBOM set already +implicitly contains. + +Two independent consumers pay this cost today: + +- A reporting tool infers the group **heuristically**, from the `sourceCodeReferences` paths of + the components. That is not merely inelegant, it is ambiguous: `sourceCodeReferences` is + collected model-wide, so a package in repo A lists files in repo B, and the group has to be + recovered by majority vote over path segments. For a repository mirrored into two groups the + vote is **unresolvable** — both SBOMs vote for the same winner and the two locations cannot be + told apart. +- A downstream integration downloads the model for nothing else. + +The path is not expensive to publish. The generator already computes it and already depends on +it — the serial number is derived from it. + +### The identity that is missing is the only stable one + +Mirrored repositories do not share a `bom-ref`, as an earlier draft of this design assumed. The +collision suffix in `_multi_sbom_context` gives them distinct refs: + +``` +shared | bom-ref: shared | serial: urn:uuid:905b7d45... +shared | bom-ref: shared-2 | serial: urn:uuid:71368b41... +``` + +That suffix is assigned in **traversal order**. Delete `GroupA`'s copy and `GroupB`'s silently +becomes `shared`. So `bom-ref` distinguishes the two documents but does not identify either one +across time, `name` identifies neither, and `serialNumber` is an opaque hash. After this change +`softagram:elementPath` is the only stable, human-legible per-element identity in the document. + +--- + +## 2. Spec + +### Target output + +```json +{ "bom-ref": "repoa", + "name": "repoA", + "group": "/OrgName/GroupA", + "type": "application", + "version": "", + "purl": "", + "externalReferences": [ + { "url": "https://example.org/org/repoA.git", "type": "vcs" } + ], + "properties": [ + { "name": "softagram:elementPath", "value": "/OrgName/GroupA/repoA" } + ] } +``` + +### Field by field + +| Field | Value | Why | +|---|---|---| +| `group` | the parent element's **full path**, omitted when that path is empty | Native CycloneDX field, so no custom-property parsing is needed for the common case. Holding the full parent path rather than the bare parent name keeps two identically named groups under different roots distinguishable, and hands the consumer the estate root without a second lookup. | +| `properties[softagram:elementPath]` | full path from the model root | Exact and lossless: works at any `--level`, survives deeper nesting, and is the single value from which the whole tree can be rebuilt. | +| `externalReferences[type=vcs]` | `repo_url` of the element, or of the nearest ancestor that has one; absent when none does | CycloneDX has a proper field for the repository URL, and the model already carries it. Removes another reason to fetch the model. | +| `purl` | stays empty | See below. | +| `bom-ref` | **unchanged** | See non-goals. | + +`group` and `elementPath` are strictly redundant of each other — `group` is `elementPath` minus +its last segment. That is accepted deliberately: `group` is the field a stock CycloneDX consumer +reads with no custom-property support, and `elementPath` is the exact string +`deterministic_serial()` hashes, so it must be published verbatim rather than reconstructed. + +### Why the path must not go into `purl` + +purl has a grammar: `pkg:type/namespace/name@version`, where the type must start with a letter +and hold only `[a-zA-Z0-9.+-]`. `/OrgName/GroupA/repoA` is not a purl and cannot be made into one +by placing it in that field. A purl-parsing consumer would reject or mis-parse the component — +the same failure class as the historical `pkg:???` placeholder that 1.7.1 removed for exactly +this reason. + +So `purl` stays empty until there is a real package identity to put there. + +### Why a property rather than a new top-level field + +The CycloneDX 1.7 component schema sets **`additionalProperties: false`**. A field such as +`elementPath` on the component would make the document schema-invalid. `properties` is the +schema's own extension point and therefore the only lawful place for the full path. + +`group`, by contrast, is a native field and needs no extension point. It is currently unused +anywhere in the module — Maven `groupId` goes into the purl namespace, not into `group` — so +there is no collision with an existing meaning. + +### Naming + +`softagram:elementPath` follows the namespace the module's two newest extensions use — +`softagram:via` and `softagram:internal`. The module is not consistent here: `purlTypeResolution`, +`versionSource` and `sourceCodeReferences` carry no prefix. New names take the prefix; renaming +the existing ones is a separate, breaking decision. + +### Invariants worth asserting + +1. `deterministic_serial(elementPath) == serialNumber` — the published path must be the same + string the serial was derived from. This turns the property from decoration into something + verifiable, and would catch a future refactor that computes one from a normalised path and the + other from the raw one. +2. `group + '/' + name == elementPath` at every level below the top, and `group` is absent at the + top level. +3. Two SBOMs for one mirrored repository share `name`, carry **distinct** `bom-ref`s (via the + traversal-order collision suffix) and distinct `serialNumber`s, and are told apart + unambiguously by `group` and `elementPath`. +4. A vcs reference is present exactly when the element or one of its ancestors carries + `repo_url`. No fabricated URL is ever emitted on the new code paths. + +### A caveat for consumers + +The first path segment is the model's estate root and **is not stable** — it changes when the +estate is renamed or restructured. Consumers should read it from the path rather than hardcode +it. `group` moves with it, so a consumer comparing `group` across model generations must expect +the same. + +--- + +## 3. Non-goals + +- **Do not change `bom-ref`.** It is referenced from `dependencies[].dependsOn` and from BOM-Link + URNs in other SBOMs. Changing it breaks cross-SBOM resolution for every already-ingested + document. (`analyze_component_section` already uses a path-valued `bom-ref`; that is not a + reason to change this one.) +- **Do not change `serialNumber`.** It is already derived from the path and must stay stable so + re-ingesting a model updates projects rather than creating new ones. +- **Do not fill `version`.** A repository has no version; a commit hash is not one either. +- **Do not touch the `UNKNOWN-REPOSITORY_LOCATION` fallback** in `analyze_component_section`. + Fabricating a URL for a repository whose remote is unknown is a defect, but removing it changes + existing output and is its own decision. The new code paths never fabricate. +- **Do not remove the model download from every workflow.** This change removes it for tree + reconstruction. Measuring which repositories the analyzer failed to read still requires the + model, because that fact is absent from the SBOMs by construction. + +--- + +## 4. Implementation + +### Two helpers + +Both mutate an already-built component dict, matching the idiom already used for +`softagram:via`: + +```python +ELEMENT_PATH_PROPERTY = 'softagram:elementPath' + + +def _add_element_location(component, elem): + """Publish where elem sits in the model, on a component that describes elem. + + 'group' carries the parent's full path rather than its bare name so two identically named + groups under different roots stay distinguishable, and is omitted at the top level, where + there is no parent to name. The element's own path goes into a property because CycloneDX + sets additionalProperties: false on component; it is also the exact string + deterministic_serial() hashes, so a consumer can verify the identity without the model. + """ + parent_path = elem.parent.getPath() if elem.parent is not None else '' + if parent_path: + component['group'] = parent_path + component.setdefault('properties', []).append( + {'name': ELEMENT_PATH_PROPERTY, 'value': elem.getPath()}) + + +def _add_vcs_reference(component, elem): + """Publish the repository URL of elem, or of the nearest ancestor carrying one. + + A directory-level SBOM belongs to its repository's VCS, so the walk upwards is what makes + the field correct there rather than merely absent. Nothing is emitted when no ancestor has + a repo_url: an invented URL is worse than a missing one, because a consumer cannot tell it + from a real one. + """ + while elem is not None: + if 'repo_url' in elem.attrs: + component.setdefault('externalReferences', []).append( + {'url': elem.attrs['repo_url'], 'type': 'vcs'}) + return + elem = elem.parent +``` + +`getPath()` returns `''` for the model root and for a level-1 element's parent, so +`if parent_path:` is the entire top-level guard — no `parent is None` special case is needed for +the omission, only for safety when an element is detached. + +### Three call sites + +`metadata_component` is assembled in more than one place, and a third site describes model +elements without being a metadata component. Every site that describes a **model element** gets +both fields; a site that describes something without a model path gets neither. + +| Site | What it describes | Gets | +|---|---|---| +| `_sbom_for_content_element` (~:1045) | the metadata component of a `--level` or `--element-path` SBOM | location + vcs | +| transitive internal components (~:939) | reachable internal elements inlined into one BOM | location + vcs | +| `analyze_component_section` (~:820) | the level-1 element of the legacy single SBOM | location only; `group` self-omits | + +Both public entry points — `generate_multi_from_sgraph` and `generate_for_element_from_sgraph` — +route through `_sbom_for_content_element`, so one edit there covers both. The legacy path keeps +its own existing vcs logic, including the fabricated-URL fallback, per non-goals. + +3rd-party components from the External subtree get neither field: they describe packages, not +model elements, and already carry `sourceCodeReferences`. + +### Tests + +TDD order: characterization tests pinning current output first, then the feature. + +Against `modelfile_for_sbom_multi_tests.xml` (paths `/OrgName/GroupA/repoA`, `/OrgName/GroupA/repoB`), +using the `find_property` helper already in the test module: + +| Test | Asserts | +|---|---| +| `test_metadata_component_carries_element_path` | property equals `/OrgName/GroupA/repoA` | +| `test_metadata_component_carries_parent_group` | `group` equals `/OrgName/GroupA` | +| `test_element_path_matches_serial_number` | `deterministic_serial(path) == serialNumber`, for every SBOM | +| `test_element_path_at_level_2` | at `--level 2` the group is `/OrgName`, proving the fields are level-agnostic | +| `test_group_is_absent_at_the_top_level` | at `--level 1` no `group` key is emitted | +| `test_purl_stays_empty` | `purl` is still `''` — guards against a later "fill the empty field" edit | +| `test_bom_ref_unchanged` | still the slug, not the path | +| `test_transitive_internal_component_carries_its_location` | the inlined `repoB` component carries `group` and `elementPath` | +| `test_vcs_reference_from_the_elements_own_repo_url` | `repoA`'s SBOM carries its `repo_url` as a vcs reference | +| `test_vcs_reference_inherited_from_the_nearest_ancestor` | a dir-level `--element-path` SBOM inherits the repo's URL | +| `test_no_vcs_reference_when_no_ancestor_has_one` | absent, not fabricated | +| `test_legacy_single_sbom_carries_the_element_path` | `analyze_component_section` publishes the path and omits `group` | + +`repo_url` is added to `repoA` in the existing multi fixture. This is safe: the one existing test +asserting on those `externalReferences` filters `type == 'bom'` first. + +A new fixture `modelfile_for_sbom_mirrored_tests.xml` carries one repository name under two +groups — the case the feature exists to disambiguate, which no current fixture covers: + +| Test | Asserts | +|---|---| +| `test_mirrored_repositories_are_distinguished_by_location` | same `name`, distinct `group`, `elementPath` and `serialNumber` | + +### Verification against a real model + +```bash +.venv/bin/python -m sgraph.converters.sbom_cyclonedx_generator \ + model.xml sboms_level3.json --level 3 +``` + +then check the invariants hold across every SBOM: + +```python +import json +from sgraph.converters.sbom_cyclonedx_generator import deterministic_serial + +sboms = json.load(open('sboms_level3.json')) +for s in sboms: + c = s['metadata']['component'] + path = next(p['value'] for p in c['properties'] + if p['name'] == 'softagram:elementPath') + assert deterministic_serial(path) == s['serialNumber'], c['name'] + assert path == c['group'] + '/' + c['name'], c['name'] +print(len(sboms), 'SBOMs: elementPath consistent with serial and group') +``` + +Expected cost: additive fields only, so component counts, purls and serials must be +**byte-identical** to the previous run apart from the new fields and the timestamp. Any other +difference means something unintended moved. + +--- + +## 5. Consumer impact + +- **Additive.** No existing field changes value, so nothing already ingested breaks. +- The integration that motivated this drops the model download entirely. +- The reporting tool can delete its group-inference heuristic and read `group` and the estate + straight from the metadata component — which also fixes the mirrored-repository ambiguity it + currently cannot resolve. +- Transitive-mode consumers gain the location of every link in the exposure chain, not just its + root. From 1985b3d28ba64f6bd00377b4fd548a8b87ace19a Mon Sep 17 00:00:00 2001 From: Ville Laitila Date: Tue, 11 Aug 2026 17:05:06 +0300 Subject: [PATCH 02/11] test: pin the SBOM metadata component fields that must not change --- .../sbom_cyclonedx_generator_test.py | 44 +++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/tests/converters/sbom_cyclonedx_generator_test.py b/tests/converters/sbom_cyclonedx_generator_test.py index 4eb0822..36aeb37 100644 --- a/tests/converters/sbom_cyclonedx_generator_test.py +++ b/tests/converters/sbom_cyclonedx_generator_test.py @@ -439,6 +439,50 @@ def test_cli_rejects_transitive_without_level(tmp_path): assert '--transitive requires --level' in proc.stderr +# --- Element location tests --- +# +# The model position of an element is published two ways: the native CycloneDX 'group' field +# (the parent's full path) and a 'softagram:elementPath' property (the element's own path). +# The property is the exact string deterministic_serial() hashes, so it is verifiable. + + +def test_purl_and_version_stay_empty_on_the_metadata_component(): + """Characterization: the metadata component has no package identity, and must not gain one. + + Guards against a later 'fill the empty field' edit putting the element path into purl, where + it would violate the purl grammar and be rejected by a purl-parsing consumer. + """ + model, _ = get_model_and_model_api(MULTI_MODEL) + result = generate_multi_from_sgraph(model, level=3) + + assert len(result) == 2 + for sbom in result: + assert sbom['metadata']['component']['purl'] == '' + assert sbom['metadata']['component']['version'] == '' + + +def test_bom_ref_stays_the_slug_not_the_path(): + """Characterization: bom-ref is referenced from other SBOMs and must not become the path.""" + model, _ = get_model_and_model_api(MULTI_MODEL) + result = generate_multi_from_sgraph(model, level=3) + + refs = sorted(sbom['metadata']['component']['bom-ref'] for sbom in result) + assert refs == ['repoa', 'repob'] + + +def test_serial_numbers_stay_derived_from_the_element_path(): + """Characterization: serialNumber == deterministic_serial(element_path), and stays that way. + + Re-ingesting a model must update the same projects rather than create new ones. + """ + model, _ = get_model_and_model_api(MULTI_MODEL) + result = generate_multi_from_sgraph(model, level=3) + + serials = {sbom['metadata']['component']['name']: sbom['serialNumber'] for sbom in result} + assert serials['repoA'] == deterministic_serial('/OrgName/GroupA/repoA') + assert serials['repoB'] == deterministic_serial('/OrgName/GroupA/repoB') + + # --- purl type inference tests --- BINARY_REFS_MODEL = 'converters/modelfile_for_sbom_binary_refs_tests.xml' From 06a68fabbd3db912ba6752deafd9ae73a2bad560 Mon Sep 17 00:00:00 2001 From: Ville Laitila Date: Tue, 11 Aug 2026 18:11:22 +0300 Subject: [PATCH 03/11] SBOM: publish the element's model path and parent path on the metadata component --- .../converters/sbom_cyclonedx_generator.py | 31 +++++++++ .../sbom_cyclonedx_generator_test.py | 66 +++++++++++++++++++ 2 files changed, 97 insertions(+) diff --git a/src/sgraph/converters/sbom_cyclonedx_generator.py b/src/sgraph/converters/sbom_cyclonedx_generator.py index 2a25955..9e47442 100644 --- a/src/sgraph/converters/sbom_cyclonedx_generator.py +++ b/src/sgraph/converters/sbom_cyclonedx_generator.py @@ -762,6 +762,36 @@ def analyze_3rdparty(external_root, sbom): stack += elem.children +ELEMENT_PATH_PROPERTY = 'softagram:elementPath' + + +def _add_element_location(component, elem): + """Publish where elem sits in the model, on a component that describes elem. + + 'group' carries the parent's full path rather than its bare name so that two identically + named groups under different roots stay distinguishable, and is omitted for a top-level + element, whose parent is the model root and has no path of its own. + + The element's own path goes into a property because CycloneDX sets additionalProperties: + false on component, leaving properties[] as the only schema-valid place for it. It is also + the exact string deterministic_serial() hashes, so publishing it verbatim lets a consumer + verify the document's identity without loading the model. + + Call once per component: this overwrites 'group' and appends to 'properties'. elem must be + attached to a model. A detached element is not merely unsupported here, it is dangerous: + getPath() would return a bare name with no leading slash, and publishing that as an + elementPath yields a document a consumer cannot resolve and cannot detect as broken. + Raising at the call site is the better failure. + """ + parent_path = elem.parent.getPath() + if parent_path: + component['group'] = parent_path + component.setdefault('properties', []).append({ + 'name': ELEMENT_PATH_PROPERTY, + 'value': elem.getPath() + }) + + class SBOM: BASIC_INFO = { @@ -1050,6 +1080,7 @@ def _sbom_for_content_element(orig_elem, ctx, transitive): 'purl': '', 'externalReferences': [] } + _add_element_location(sbom.metadata_component, orig_elem) if transitive: sbom.components, dependencies = _transitive_components_and_dependencies( diff --git a/tests/converters/sbom_cyclonedx_generator_test.py b/tests/converters/sbom_cyclonedx_generator_test.py index 36aeb37..83ff321 100644 --- a/tests/converters/sbom_cyclonedx_generator_test.py +++ b/tests/converters/sbom_cyclonedx_generator_test.py @@ -483,6 +483,72 @@ def test_serial_numbers_stay_derived_from_the_element_path(): assert serials['repoB'] == deterministic_serial('/OrgName/GroupA/repoB') +def test_metadata_component_carries_element_path(): + """The full model path is published as a property, because CycloneDX has no field for it.""" + model, _ = get_model_and_model_api(MULTI_MODEL) + result = generate_multi_from_sgraph(model, level=3) + + component = sbom_of(result, 'repoA')['metadata']['component'] + assert find_property(component, 'softagram:elementPath') == '/OrgName/GroupA/repoA' + + +def test_metadata_component_carries_the_parent_path_as_group(): + """group holds the parent's full path, so two same-named groups stay distinguishable.""" + model, _ = get_model_and_model_api(MULTI_MODEL) + result = generate_multi_from_sgraph(model, level=3) + + assert sbom_of(result, 'repoA')['metadata']['component']['group'] == '/OrgName/GroupA' + assert sbom_of(result, 'repoB')['metadata']['component']['group'] == '/OrgName/GroupA' + + +def test_element_path_matches_the_serial_number_for_every_sbom(): + """The published path must be the same string the serial was derived from. + + This is the invariant that makes the property verifiable rather than decorative: it catches a + future refactor that derives one from a normalised path and the other from the raw one. + """ + model, _ = get_model_and_model_api(MULTI_MODEL) + result = generate_multi_from_sgraph(model, level=3) + + assert len(result) == 2 + for sbom in result: + path = find_property(sbom['metadata']['component'], 'softagram:elementPath') + assert deterministic_serial(path) == sbom['serialNumber'] + + +def test_element_location_is_level_agnostic(): + """At level 2 the element is the group itself, and its parent is the estate root.""" + model, _ = get_model_and_model_api(MULTI_MODEL) + result = generate_multi_from_sgraph(model, level=2) + + component = sbom_of(result, 'GroupA')['metadata']['component'] + assert component['group'] == '/OrgName' + assert find_property(component, 'softagram:elementPath') == '/OrgName/GroupA' + + +def test_group_is_absent_at_the_top_level(): + """A top-level element has no parent to name, so the key is omitted rather than emitted empty. + + An empty group cannot be told apart from 'the tool forgot to fill it in'; an absent one can. + """ + model, _ = get_model_and_model_api(MULTI_MODEL) + result = generate_multi_from_sgraph(model, level=1) + + component = sbom_of(result, 'OrgName')['metadata']['component'] + assert 'group' not in component + assert find_property(component, 'softagram:elementPath') == '/OrgName' + + +def test_selected_element_sbom_also_carries_its_location(): + """--element-path routes through the same helper as --level, so it gets the same fields.""" + model, _ = get_model_and_model_api(MULTI_MODEL) + sbom = generate_for_element_from_sgraph(model, '/OrgName/GroupA/repoA/src') + + component = sbom['metadata']['component'] + assert component['group'] == '/OrgName/GroupA/repoA' + assert find_property(component, 'softagram:elementPath') == '/OrgName/GroupA/repoA/src' + + # --- purl type inference tests --- BINARY_REFS_MODEL = 'converters/modelfile_for_sbom_binary_refs_tests.xml' From a2d48db5dddcda7b9fd9769f825ad58068449c99 Mon Sep 17 00:00:00 2001 From: Ville Laitila Date: Tue, 11 Aug 2026 18:24:18 +0300 Subject: [PATCH 04/11] docs: drop the unreachable parent-is-None fallback from the helper A detached element would have published its bare name as elementPath - a value that looks like a path but resolves to nothing. Raising at the call site is the better failure. --- .../2026-08-11-sbom-element-path-design.md | 31 +++++++++++++------ 1 file changed, 21 insertions(+), 10 deletions(-) diff --git a/docs/superpowers/specs/2026-08-11-sbom-element-path-design.md b/docs/superpowers/specs/2026-08-11-sbom-element-path-design.md index 0b26940..ca6f2eb 100644 --- a/docs/superpowers/specs/2026-08-11-sbom-element-path-design.md +++ b/docs/superpowers/specs/2026-08-11-sbom-element-path-design.md @@ -168,16 +168,25 @@ def _add_element_location(component, elem): """Publish where elem sits in the model, on a component that describes elem. 'group' carries the parent's full path rather than its bare name so two identically named - groups under different roots stay distinguishable, and is omitted at the top level, where - there is no parent to name. The element's own path goes into a property because CycloneDX - sets additionalProperties: false on component; it is also the exact string - deterministic_serial() hashes, so a consumer can verify the identity without the model. + groups under different roots stay distinguishable, and is omitted for a top-level element, + whose parent is the model root and has no path of its own. The element's own path goes into + a property because CycloneDX sets additionalProperties: false on component; it is also the + exact string deterministic_serial() hashes, so a consumer can verify the identity without + the model. + + Call once per component: this overwrites 'group' and appends to 'properties'. elem must be + attached to a model. A detached element is not merely unsupported here, it is dangerous: + getPath() would return a bare name with no leading slash, and publishing that as an + elementPath yields a document a consumer cannot resolve and cannot detect as broken. + Raising at the call site is the better failure. """ - parent_path = elem.parent.getPath() if elem.parent is not None else '' + parent_path = elem.parent.getPath() if parent_path: component['group'] = parent_path - component.setdefault('properties', []).append( - {'name': ELEMENT_PATH_PROPERTY, 'value': elem.getPath()}) + component.setdefault('properties', []).append({ + 'name': ELEMENT_PATH_PROPERTY, + 'value': elem.getPath() + }) def _add_vcs_reference(component, elem): @@ -196,9 +205,11 @@ def _add_vcs_reference(component, elem): elem = elem.parent ``` -`getPath()` returns `''` for the model root and for a level-1 element's parent, so -`if parent_path:` is the entire top-level guard — no `parent is None` special case is needed for -the omission, only for safety when an element is detached. +`getPath()` returns `''` for the model root, which is a level-1 element's parent, so +`if parent_path:` is the entire top-level guard. There is deliberately **no** `parent is None` +fallback: it is unreachable from every call site, and where it would fire it would emit +`elementPath` as a bare name with no leading slash — a value that looks like a path but resolves +to nothing. Raising beats publishing that. ### Three call sites From 64ae65bd28b6068157e3452e5639b1e1fa285d0c Mon Sep 17 00:00:00 2001 From: Ville Laitila Date: Tue, 11 Aug 2026 18:26:42 +0300 Subject: [PATCH 05/11] test: prove element location disambiguates a mirrored repository --- .../modelfile_for_sbom_mirrored_tests.xml | 32 +++++++++++++++++++ .../sbom_cyclonedx_generator_test.py | 32 +++++++++++++++++++ 2 files changed, 64 insertions(+) create mode 100644 tests/converters/modelfile_for_sbom_mirrored_tests.xml diff --git a/tests/converters/modelfile_for_sbom_mirrored_tests.xml b/tests/converters/modelfile_for_sbom_mirrored_tests.xml new file mode 100644 index 0000000..a85a974 --- /dev/null +++ b/tests/converters/modelfile_for_sbom_mirrored_tests.xml @@ -0,0 +1,32 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/tests/converters/sbom_cyclonedx_generator_test.py b/tests/converters/sbom_cyclonedx_generator_test.py index 83ff321..cb87f6b 100644 --- a/tests/converters/sbom_cyclonedx_generator_test.py +++ b/tests/converters/sbom_cyclonedx_generator_test.py @@ -549,6 +549,38 @@ def test_selected_element_sbom_also_carries_its_location(): assert find_property(component, 'softagram:elementPath') == '/OrgName/GroupA/repoA/src' +MIRRORED_MODEL = 'converters/modelfile_for_sbom_mirrored_tests.xml' + + +def test_mirrored_repositories_are_distinguished_by_their_location(): + """One repository name under two groups: the only field that tells them apart is the path. + + 'name' is identical. 'bom-ref' differs only through a traversal-order collision suffix, so it + is not a stable identity: drop GroupA's copy and GroupB's silently becomes 'shared'. + 'serialNumber' differs but is an opaque hash. group and elementPath are the answer. + """ + model, _ = get_model_and_model_api(MIRRORED_MODEL) + result = generate_multi_from_sgraph(model, level=3) + + components = [sbom['metadata']['component'] for sbom in result] + assert [c['name'] for c in components] == ['shared', 'shared'] + + by_group = {c['group']: c for c in components} + assert sorted(by_group) == ['/OrgName/GroupA', '/OrgName/GroupB'] + + assert find_property(by_group['/OrgName/GroupA'], 'softagram:elementPath') \ + == '/OrgName/GroupA/shared' + assert find_property(by_group['/OrgName/GroupB'], 'softagram:elementPath') \ + == '/OrgName/GroupB/shared' + + # The collision suffix keeps bom-refs unique within the set, but does not identify either one + assert sorted(c['bom-ref'] for c in components) == ['shared', 'shared-2'] + + # Mirror-specific: a serial derived from the NAME rather than the path would collide here, + # where the names are identical. The generic path/serial invariant is pinned elsewhere. + assert len({sbom['serialNumber'] for sbom in result}) == 2 + + # --- purl type inference tests --- BINARY_REFS_MODEL = 'converters/modelfile_for_sbom_binary_refs_tests.xml' From 31cf8a49d99b045fbec069d83768500a5baa4c76 Mon Sep 17 00:00:00 2001 From: Ville Laitila Date: Tue, 11 Aug 2026 18:37:36 +0300 Subject: [PATCH 06/11] SBOM: publish the repository URL as a vcs external reference --- .../converters/sbom_cyclonedx_generator.py | 28 +++++++ .../modelfile_for_sbom_mirrored_tests.xml | 2 +- .../modelfile_for_sbom_multi_tests.xml | 2 +- .../sbom_cyclonedx_generator_test.py | 73 +++++++++++++++++++ 4 files changed, 103 insertions(+), 2 deletions(-) diff --git a/src/sgraph/converters/sbom_cyclonedx_generator.py b/src/sgraph/converters/sbom_cyclonedx_generator.py index 9e47442..3ac8b5b 100644 --- a/src/sgraph/converters/sbom_cyclonedx_generator.py +++ b/src/sgraph/converters/sbom_cyclonedx_generator.py @@ -792,6 +792,33 @@ def _add_element_location(component, elem): }) +def _add_vcs_reference(component, elem): + """Publish the repository URL of elem, or of the nearest ancestor carrying one. + + The walk upwards is what makes the field correct rather than merely absent on a + directory-level SBOM: the directory has no remote of its own, but the repository it lives + in does, and that is the VCS location of its contents. The NEAREST carrier wins, because a + sub-repo's own remote describes it better than its estate's does. + + A blank attribute is not an answer, so the walk continues past it. Stopping there would + publish an empty url and, worse, suppress a real remote one level further up. + + Nothing is emitted when no ancestor carries a usable repo_url. An invented URL is worse + than a missing one, because a consumer cannot tell it from a real one. + + Call this only for components describing INTERNAL model elements. Were it run against a + 3rd-party component in the External subtree, the walk would climb out to the estate root + and attribute that package to the analyzed organisation's own repository. + """ + ancestor = elem + while ancestor is not None: + repo_url = ancestor.attrs.get('repo_url', '').strip() + if repo_url: + component.setdefault('externalReferences', []).append({'url': repo_url, 'type': 'vcs'}) + return + ancestor = ancestor.parent + + class SBOM: BASIC_INFO = { @@ -1081,6 +1108,7 @@ def _sbom_for_content_element(orig_elem, ctx, transitive): 'externalReferences': [] } _add_element_location(sbom.metadata_component, orig_elem) + _add_vcs_reference(sbom.metadata_component, orig_elem) if transitive: sbom.components, dependencies = _transitive_components_and_dependencies( diff --git a/tests/converters/modelfile_for_sbom_mirrored_tests.xml b/tests/converters/modelfile_for_sbom_mirrored_tests.xml index a85a974..fb5c559 100644 --- a/tests/converters/modelfile_for_sbom_mirrored_tests.xml +++ b/tests/converters/modelfile_for_sbom_mirrored_tests.xml @@ -1,7 +1,7 @@ - + diff --git a/tests/converters/modelfile_for_sbom_multi_tests.xml b/tests/converters/modelfile_for_sbom_multi_tests.xml index 261b897..8ceab54 100644 --- a/tests/converters/modelfile_for_sbom_multi_tests.xml +++ b/tests/converters/modelfile_for_sbom_multi_tests.xml @@ -2,7 +2,7 @@ - + diff --git a/tests/converters/sbom_cyclonedx_generator_test.py b/tests/converters/sbom_cyclonedx_generator_test.py index cb87f6b..7d08b6f 100644 --- a/tests/converters/sbom_cyclonedx_generator_test.py +++ b/tests/converters/sbom_cyclonedx_generator_test.py @@ -581,6 +581,79 @@ def test_mirrored_repositories_are_distinguished_by_their_location(): assert len({sbom['serialNumber'] for sbom in result}) == 2 +def test_vcs_reference_comes_from_the_elements_own_repo_url(): + """The model already holds the repository URL; CycloneDX has a proper field for it.""" + model, _ = get_model_and_model_api(MULTI_MODEL) + result = generate_multi_from_sgraph(model, level=3) + + component = sbom_of(result, 'repoA')['metadata']['component'] + vcs = [r['url'] for r in component['externalReferences'] if r['type'] == 'vcs'] + assert vcs == ['https://example.org/org/repoA.git'] + + +def test_vcs_reference_is_inherited_from_the_nearest_ancestor(): + """A directory-level SBOM belongs to its repository's VCS, one level up.""" + model, _ = get_model_and_model_api(MULTI_MODEL) + sbom = generate_for_element_from_sgraph(model, '/OrgName/GroupA/repoA/src') + + vcs = [r['url'] for r in sbom['metadata']['component']['externalReferences'] + if r['type'] == 'vcs'] + assert vcs == ['https://example.org/org/repoA.git'] + + +def test_no_vcs_reference_when_no_ancestor_has_one(): + """Absent, never invented: a consumer cannot tell a fabricated URL from a real one.""" + model, _ = get_model_and_model_api(MULTI_MODEL) + result = generate_multi_from_sgraph(model, level=3) + + component = sbom_of(result, 'repoB')['metadata']['component'] + assert [r for r in component['externalReferences'] if r['type'] == 'vcs'] == [] + + +def test_mirrored_repositories_carry_their_own_distinct_repository_urls(): + """Two mirrors of one name are two different repositories, each with its own remote.""" + model, _ = get_model_and_model_api(MIRRORED_MODEL) + result = generate_multi_from_sgraph(model, level=3) + + by_group = {sbom['metadata']['component']['group']: sbom['metadata']['component'] + for sbom in result} + assert [r['url'] for r in by_group['/OrgName/GroupA']['externalReferences'] + if r['type'] == 'vcs'] == ['https://example.org/org/groupa-shared.git'] + assert [r['url'] for r in by_group['/OrgName/GroupB']['externalReferences'] + if r['type'] == 'vcs'] == ['https://example.org/org/groupb-shared.git'] + + +def test_nearest_repo_url_wins_over_a_more_distant_ancestor(): + """A sub-repo's own remote describes it; its parent group's does not. + + Without this, a farthest-ancestor-wins regression passes the whole suite: no other fixture + has two repo_url attributes on one chain, so nothing else can tell the two rules apart. + """ + model, _ = get_model_and_model_api(MIRRORED_MODEL) + result = generate_multi_from_sgraph(model, level=3) + + component = next(s['metadata']['component'] for s in result + if s['metadata']['component']['group'] == '/OrgName/GroupA') + vcs = [r['url'] for r in component['externalReferences'] if r['type'] == 'vcs'] + assert vcs == ['https://example.org/org/groupa-shared.git'] + + +def test_a_blank_repo_url_does_not_mask_a_real_one_further_up(): + """A blank attribute is not an answer. Publishing it would also hide a recoverable URL.""" + model = SGraph(SElement(None, '')) + model.createOrGetElementFromPath('/Org/repo/sub') + model.findElementFromPath('/Org/repo').attrs['repo_url'] = 'https://example.org/repo.git' + model.findElementFromPath('/Org/repo/sub').attrs['repo_url'] = ' ' + + component = {} + sbom_cyclonedx_generator._add_vcs_reference( + component, model.findElementFromPath('/Org/repo/sub')) + + assert component['externalReferences'] == [ + {'url': 'https://example.org/repo.git', 'type': 'vcs'} + ] + + # --- purl type inference tests --- BINARY_REFS_MODEL = 'converters/modelfile_for_sbom_binary_refs_tests.xml' From 5a3601df0af560d822856774acbc919875f08958 Mon Sep 17 00:00:00 2001 From: Ville Laitila Date: Tue, 11 Aug 2026 19:01:58 +0300 Subject: [PATCH 07/11] docs: sync with Task 4 review fixes (blank repo_url masking, nearest-wins) --- docs/example_deps.txt | 2 + .../2026-08-11-sbom-element-path-design.md | 37 +++++++++++++------ 2 files changed, 28 insertions(+), 11 deletions(-) create mode 100644 docs/example_deps.txt diff --git a/docs/example_deps.txt b/docs/example_deps.txt new file mode 100644 index 0000000..df1af2b --- /dev/null +++ b/docs/example_deps.txt @@ -0,0 +1,2 @@ +/Software complexity:/Developer anxiety:cause +/Software size:/Maintenance cost:/cause diff --git a/docs/superpowers/specs/2026-08-11-sbom-element-path-design.md b/docs/superpowers/specs/2026-08-11-sbom-element-path-design.md index ca6f2eb..eb24a8e 100644 --- a/docs/superpowers/specs/2026-08-11-sbom-element-path-design.md +++ b/docs/superpowers/specs/2026-08-11-sbom-element-path-design.md @@ -123,8 +123,10 @@ the existing ones is a separate, breaking decision. 3. Two SBOMs for one mirrored repository share `name`, carry **distinct** `bom-ref`s (via the traversal-order collision suffix) and distinct `serialNumber`s, and are told apart unambiguously by `group` and `elementPath`. -4. A vcs reference is present exactly when the element or one of its ancestors carries - `repo_url`. No fabricated URL is ever emitted on the new code paths. +4. A vcs reference is present exactly when the element or one of its ancestors carries a + **non-blank** `repo_url`, and carries the value of the **nearest** such ancestor. A blank + attribute is walked past rather than published: stopping on it would emit an empty url and + suppress a real remote further up. No fabricated URL is ever emitted on the new code paths. ### A caveat for consumers @@ -192,17 +194,28 @@ def _add_element_location(component, elem): def _add_vcs_reference(component, elem): """Publish the repository URL of elem, or of the nearest ancestor carrying one. - A directory-level SBOM belongs to its repository's VCS, so the walk upwards is what makes - the field correct there rather than merely absent. Nothing is emitted when no ancestor has - a repo_url: an invented URL is worse than a missing one, because a consumer cannot tell it - from a real one. + The walk upwards is what makes the field correct rather than merely absent on a + directory-level SBOM: the directory has no remote of its own, but the repository it lives + in does, and that is the VCS location of its contents. The NEAREST carrier wins, because a + sub-repo's own remote describes it better than its estate's does. + + A blank attribute is not an answer, so the walk continues past it. Stopping there would + publish an empty url and, worse, suppress a real remote one level further up. + + Nothing is emitted when no ancestor carries a usable repo_url. An invented URL is worse + than a missing one, because a consumer cannot tell it from a real one. + + Call this only for components describing INTERNAL model elements. Were it run against a + 3rd-party component in the External subtree, the walk would climb out to the estate root + and attribute that package to the analyzed organisation's own repository. """ - while elem is not None: - if 'repo_url' in elem.attrs: - component.setdefault('externalReferences', []).append( - {'url': elem.attrs['repo_url'], 'type': 'vcs'}) + ancestor = elem + while ancestor is not None: + repo_url = ancestor.attrs.get('repo_url', '').strip() + if repo_url: + component.setdefault('externalReferences', []).append({'url': repo_url, 'type': 'vcs'}) return - elem = elem.parent + ancestor = ancestor.parent ``` `getPath()` returns `''` for the model root, which is a level-1 element's parent, so @@ -250,6 +263,8 @@ using the `find_property` helper already in the test module: | `test_vcs_reference_from_the_elements_own_repo_url` | `repoA`'s SBOM carries its `repo_url` as a vcs reference | | `test_vcs_reference_inherited_from_the_nearest_ancestor` | a dir-level `--element-path` SBOM inherits the repo's URL | | `test_no_vcs_reference_when_no_ancestor_has_one` | absent, not fabricated | +| `test_nearest_repo_url_wins_over_a_more_distant_ancestor` | the nearest carrier wins — the only test that can tell nearest from farthest | +| `test_a_blank_repo_url_does_not_mask_a_real_one_further_up` | a blank value is walked past, not published | | `test_legacy_single_sbom_carries_the_element_path` | `analyze_component_section` publishes the path and omits `group` | `repo_url` is added to `repoA` in the existing multi fixture. This is safe: the one existing test From e146511ce5cd2b68a4550865a7d6cf218990603d Mon Sep 17 00:00:00 2001 From: Ville Laitila Date: Tue, 11 Aug 2026 19:03:18 +0300 Subject: [PATCH 08/11] SBOM: publish the location of every inlined internal component --- .../converters/sbom_cyclonedx_generator.py | 15 +++++-- .../sbom_cyclonedx_generator_test.py | 43 +++++++++++++++++++ 2 files changed, 54 insertions(+), 4 deletions(-) diff --git a/src/sgraph/converters/sbom_cyclonedx_generator.py b/src/sgraph/converters/sbom_cyclonedx_generator.py index 3ac8b5b..baa9669 100644 --- a/src/sgraph/converters/sbom_cyclonedx_generator.py +++ b/src/sgraph/converters/sbom_cyclonedx_generator.py @@ -990,18 +990,25 @@ def _transitive_components_and_dependencies(root_path, gen_elem_by_path, orig_el refs.append(surviving_ref_by_key[key]) external_refs_of[path] = refs - # Reachable internal elements become components of this BOM + # Reachable internal elements become components of this BOM. They describe model elements, + # so they publish their location and repository too: a consumer of one transitive BOM can + # then place every link of the exposure chain in the tree, not only the element the BOM is + # rooted at. for path in order[1:]: serial_uuid = elem_serials[path].replace('urn:uuid:', '') - components.append({ + internal_elem = orig_elem_by_path[path] + internal_component = { 'bom-ref': elem_bom_refs[path], 'type': 'library', - 'name': orig_elem_by_path[path].name, + 'name': internal_elem.name, 'version': '', 'purl': '', 'properties': [{'name': 'softagram:internal', 'value': 'true'}], 'externalReferences': [{'url': f'urn:cdx:{serial_uuid}/1', 'type': 'bom'}], - }) + } + _add_element_location(internal_component, internal_elem) + _add_vcs_reference(internal_component, internal_elem) + components.append(internal_component) # Multi-entry dependency graph: every ref resolves within this BOM dependencies = [] diff --git a/tests/converters/sbom_cyclonedx_generator_test.py b/tests/converters/sbom_cyclonedx_generator_test.py index 7d08b6f..a6e7050 100644 --- a/tests/converters/sbom_cyclonedx_generator_test.py +++ b/tests/converters/sbom_cyclonedx_generator_test.py @@ -654,6 +654,49 @@ def test_a_blank_repo_url_does_not_mask_a_real_one_further_up(): ] +def test_transitive_internal_components_carry_their_location(): + """Every link of the inlined exposure chain says where it lives, not just the chain's root.""" + model, _ = get_model_and_model_api(MULTI_MODEL) + result = generate_multi_from_sgraph(model, level=3, transitive=True) + + repo_b = next(c for c in sbom_of(result, 'repoA')['components'] if c['name'] == 'repoB') + assert repo_b['group'] == '/OrgName/GroupA' + assert find_property(repo_b, 'softagram:elementPath') == '/OrgName/GroupA/repoB' + # The pre-existing internal marker survives alongside the new property + assert find_property(repo_b, 'softagram:internal') == 'true' + # repoB has no repo_url in the fixture, so it gets no vcs reference + assert [r for r in repo_b['externalReferences'] if r['type'] == 'vcs'] == [] + + +def test_inlined_mirror_is_told_apart_from_its_host_by_its_published_location(): + """GroupA's transitive SBOM holds two components named 'shared' — one of them is itself. + + The two share a name, so a consumer reading names alone cannot tell which repository each + component is. group, elementPath and the vcs URL answer that; the bom link then resolves + the inlined one to its own standalone document. This is the case the feature exists for. + """ + model, _ = get_model_and_model_api(MIRRORED_MODEL) + result = generate_multi_from_sgraph(model, level=3, transitive=True) + + host = next(s for s in result if s['metadata']['component']['group'] == '/OrgName/GroupA') + inlined = next(c for c in host['components'] + if find_property(c, 'softagram:internal') == 'true') + + assert inlined['name'] == host['metadata']['component']['name'] == 'shared' + assert inlined['group'] == '/OrgName/GroupB' + assert find_property(inlined, 'softagram:elementPath') == '/OrgName/GroupB/shared' + + # List comprehensions rather than a {type: url} dict: these pin cardinality too, so a + # regression emitting a second vcs entry cannot pass by last-wins. + vcs = [r['url'] for r in inlined['externalReferences'] if r['type'] == 'vcs'] + assert vcs == ['https://example.org/org/groupb-shared.git'] + + mirror = next(s for s in result if s['metadata']['component']['group'] == '/OrgName/GroupB') + mirror_serial = mirror['serialNumber'].replace('urn:uuid:', '') + bom_links = [r['url'] for r in inlined['externalReferences'] if r['type'] == 'bom'] + assert bom_links == [f'urn:cdx:{mirror_serial}/1'] + + # --- purl type inference tests --- BINARY_REFS_MODEL = 'converters/modelfile_for_sbom_binary_refs_tests.xml' From 7c8e1e19f9ae6b2fae821b94f168925a914e8021 Mon Sep 17 00:00:00 2001 From: Ville Laitila Date: Tue, 11 Aug 2026 19:25:44 +0300 Subject: [PATCH 09/11] SBOM: publish the element path on the legacy single-SBOM component --- .../converters/sbom_cyclonedx_generator.py | 5 +++++ .../converters/sbom_cyclonedx_generator_test.py | 16 ++++++++++++++++ 2 files changed, 21 insertions(+) diff --git a/src/sgraph/converters/sbom_cyclonedx_generator.py b/src/sgraph/converters/sbom_cyclonedx_generator.py index baa9669..727a994 100644 --- a/src/sgraph/converters/sbom_cyclonedx_generator.py +++ b/src/sgraph/converters/sbom_cyclonedx_generator.py @@ -892,6 +892,11 @@ def analyze_component_section(elem, sbom): 'url': f'https://UNKNOWN-REPOSITORY_LOCATION/{repo.name}', 'type': 'vcs' }) + # Location only: this path builds its own vcs references from the element's children just + # above, including a fabricated placeholder when a child has no repo_url. That placeholder + # is a defect, but removing it changes output for existing consumers and is a separate + # decision — so this change neither adopts _add_vcs_reference here nor touches it. + _add_element_location(c, elem) sbom.metadata_component = c diff --git a/tests/converters/sbom_cyclonedx_generator_test.py b/tests/converters/sbom_cyclonedx_generator_test.py index a6e7050..c972c18 100644 --- a/tests/converters/sbom_cyclonedx_generator_test.py +++ b/tests/converters/sbom_cyclonedx_generator_test.py @@ -697,6 +697,22 @@ def test_inlined_mirror_is_told_apart_from_its_host_by_its_published_location(): assert bom_links == [f'urn:cdx:{mirror_serial}/1'] +def test_legacy_single_sbom_carries_the_element_path(): + """The legacy single-SBOM mode describes a model element too, so it publishes its path. + + Its element is at the top level, so group is omitted. Its bom-ref has always been the path; + that is left alone, because other documents may already reference it. + """ + model, _ = get_model_and_model_api('converters/modelfile_for_sbom_tests.xml') + sbom = sbom_cyclonedx_generator.generate_from_sgraph(model) + + component = sbom['metadata']['component'] + assert component['name'] == 'nginx' + assert find_property(component, 'softagram:elementPath') == '/nginx' + assert 'group' not in component + assert component['bom-ref'] == '/nginx' + + # --- purl type inference tests --- BINARY_REFS_MODEL = 'converters/modelfile_for_sbom_binary_refs_tests.xml' From b538ccc3de04bd3234cf363faa70831b31b56885 Mon Sep 17 00:00:00 2001 From: Ville Laitila Date: Tue, 11 Aug 2026 19:35:05 +0300 Subject: [PATCH 10/11] docs: untrack example_deps.txt, committed by accident Swept in twice by a careless 'git add docs/'. It is pre-existing local scratch output, unrelated to this change. --- docs/example_deps.txt | 2 -- 1 file changed, 2 deletions(-) delete mode 100644 docs/example_deps.txt diff --git a/docs/example_deps.txt b/docs/example_deps.txt deleted file mode 100644 index df1af2b..0000000 --- a/docs/example_deps.txt +++ /dev/null @@ -1,2 +0,0 @@ -/Software complexity:/Developer anxiety:cause -/Software size:/Maintenance cost:/cause From 1d855d4caae59aaa3545e8bc186f36961967b6f5 Mon Sep 17 00:00:00 2001 From: Ville Laitila Date: Tue, 11 Aug 2026 19:56:04 +0300 Subject: [PATCH 11/11] SBOM: document the output contract and the deliberate asymmetries Adds a CycloneDX section to docs/data-formats.md: what group and softagram:elementPath hold, the nearest-ancestor vcs rule, and the caveats a consumer needs (unstable estate root, group holds a path rather than a package namespace, inheritance by proximity). Comments three decisions that a future reader would otherwise reasonably 'fix': why 3rd-party components stay unlocated, why the ancestor walk reaches past the repository, and why the legacy path keeps its own vcs logic instead of adopting _add_vcs_reference. Excludes docs/superpowers/ from the Jekyll build - internal design notes must not publish to the GitHub Pages site. --- docs/_config.yml | 4 ++ docs/data-formats.md | 63 +++++++++++++++++++ .../2026-08-11-sbom-element-path-design.md | 54 +++++++++++----- .../converters/sbom_cyclonedx_generator.py | 24 +++++-- 4 files changed, 126 insertions(+), 19 deletions(-) diff --git a/docs/_config.yml b/docs/_config.yml index ac558bb..b86fa30 100644 --- a/docs/_config.yml +++ b/docs/_config.yml @@ -42,6 +42,10 @@ exclude: - vendor/cache/ - vendor/gems/ - vendor/ruby/ + # Internal design documents. They belong in the repository for contributors, but they are + # not user documentation and must not be published to the site. Anything user-facing that + # comes out of them belongs in a real page such as data-formats.md. + - superpowers/ # Social links author: diff --git a/docs/data-formats.md b/docs/data-formats.md index 65cc9d8..1de67d9 100644 --- a/docs/data-formats.md +++ b/docs/data-formats.md @@ -240,6 +240,69 @@ converter.convert('model.xml', 'graph.graphml') # - Cytoscape: File > Import > Network from File ``` +## CycloneDX SBOM Format + +`sgraph.converters.sbom_cyclonedx_generator` emits CycloneDX 1.7 documents. It can produce a +single SBOM for the whole model, one SBOM per element at a chosen tree depth (`--level`), or one +for a named element (`--element-path`). + +```bash +# One SBOM per repository, for a model whose repositories sit at depth 3 +python -m sgraph.converters.sbom_cyclonedx_generator model.xml sboms.json --level 3 +``` + +### Where an element lives + +Every component that describes a **model element** — the metadata component of each document, +and the internal components inlined by `--transitive` — publishes its position in the model: + +```json +{ "bom-ref": "repoa", + "name": "repoA", + "group": "/OrgName/GroupA", + "type": "application", + "version": "", + "purl": "", + "externalReferences": [ + { "url": "https://example.org/org/repoA.git", "type": "vcs" } + ], + "properties": [ + { "name": "softagram:elementPath", "value": "/OrgName/GroupA/repoA" } + ] } +``` + +| Field | Meaning | +|-------|---------| +| `group` | The **full path of the parent element**, not just its name. Omitted for a top-level element, which has no parent path. | +| `properties[softagram:elementPath]` | The element's own full path. A property rather than a field because the CycloneDX component schema sets `additionalProperties: false`. | +| `externalReferences[type=vcs]` | The `repo_url` of the element, or of the **nearest ancestor** carrying a non-blank one. Absent when no ancestor has one — never a placeholder. | + +Components describing **3rd-party packages** carry none of these. Their identity is the `purl`. + +### Guarantees + +- `deterministic_serial(elementPath) == serialNumber`. The published path is the exact string the + serial is derived from, so a consumer can verify a document's identity without the model. +- `group + '/' + name == elementPath` below the top level. +- Two repositories that share a name under different groups are distinguished by `group` and + `elementPath`. They are *not* reliably distinguished by `bom-ref`, whose collision suffix + (`repoa`, `repoa-2`) depends on traversal order and can change between model generations. + +### Caveats + +- **The first path segment is the estate root and is not stable.** It changes when the estate is + renamed or restructured. Read it from the path rather than hardcoding it. +- **`group` holds a path, not a package namespace.** The CycloneDX specification suggests + avoiding special characters in `group` and shows package coordinates such as + `org.apache.commons`. A model group is a tree location, and the slash-delimited path is what + makes two identically named groups distinguishable, so this converter prefers precision over + that convention. Tools that render `group` as a package coordinate will show the path. +- **`purl` and `version` are empty** on components describing model elements. A repository has no + package identity and no version; a path is not a valid purl and is deliberately not placed + there. +- The vcs reference is inherited by proximity. A repository with no remote of its own, under a + group that has one, reports the group's URL. + ## Format Comparison ### Performance Benchmarks diff --git a/docs/superpowers/specs/2026-08-11-sbom-element-path-design.md b/docs/superpowers/specs/2026-08-11-sbom-element-path-design.md index eb24a8e..59c5e90 100644 --- a/docs/superpowers/specs/2026-08-11-sbom-element-path-design.md +++ b/docs/superpowers/specs/2026-08-11-sbom-element-path-design.md @@ -245,37 +245,61 @@ model elements, and already carry `sourceCodeReferences`. ### Tests -TDD order: characterization tests pinning current output first, then the feature. +TDD order: characterization tests pinning current output first, then the feature. All 19 live in +one `# --- Element location tests ---` section of +`tests/converters/sbom_cyclonedx_generator_test.py`, using the `find_property` helper already in +that module. Names below are the shipped ones. -Against `modelfile_for_sbom_multi_tests.xml` (paths `/OrgName/GroupA/repoA`, `/OrgName/GroupA/repoB`), -using the `find_property` helper already in the test module: +Characterization — pin what must NOT change: + +| Test | Asserts | +|---|---| +| `test_purl_and_version_stay_empty_on_the_metadata_component` | both still `''` — guards against a later "fill the empty field" edit | +| `test_bom_ref_stays_the_slug_not_the_path` | still `repoa`/`repob`, not the path | +| `test_serial_numbers_stay_derived_from_the_element_path` | `serialNumber == deterministic_serial(element_path)` | + +Location, against `modelfile_for_sbom_multi_tests.xml` (`/OrgName/GroupA/repoA`, `.../repoB`): | Test | Asserts | |---|---| | `test_metadata_component_carries_element_path` | property equals `/OrgName/GroupA/repoA` | -| `test_metadata_component_carries_parent_group` | `group` equals `/OrgName/GroupA` | -| `test_element_path_matches_serial_number` | `deterministic_serial(path) == serialNumber`, for every SBOM | -| `test_element_path_at_level_2` | at `--level 2` the group is `/OrgName`, proving the fields are level-agnostic | +| `test_metadata_component_carries_the_parent_path_as_group` | `group` equals `/OrgName/GroupA` | +| `test_element_path_matches_the_serial_number_for_every_sbom` | `deterministic_serial(path) == serialNumber`, for every SBOM | +| `test_element_location_is_level_agnostic` | at `--level 2` the group is `/OrgName` | | `test_group_is_absent_at_the_top_level` | at `--level 1` no `group` key is emitted | -| `test_purl_stays_empty` | `purl` is still `''` — guards against a later "fill the empty field" edit | -| `test_bom_ref_unchanged` | still the slug, not the path | -| `test_transitive_internal_component_carries_its_location` | the inlined `repoB` component carries `group` and `elementPath` | -| `test_vcs_reference_from_the_elements_own_repo_url` | `repoA`'s SBOM carries its `repo_url` as a vcs reference | -| `test_vcs_reference_inherited_from_the_nearest_ancestor` | a dir-level `--element-path` SBOM inherits the repo's URL | +| `test_selected_element_sbom_also_carries_its_location` | `--element-path` gets the same fields, since it shares the helper | +| `test_legacy_single_sbom_carries_the_element_path` | `analyze_component_section` publishes the path and omits `group` | + +Repository URL: + +| Test | Asserts | +|---|---| +| `test_vcs_reference_comes_from_the_elements_own_repo_url` | `repoA`'s SBOM carries its `repo_url` as a vcs reference | +| `test_vcs_reference_is_inherited_from_the_nearest_ancestor` | a dir-level `--element-path` SBOM inherits the repo's URL | | `test_no_vcs_reference_when_no_ancestor_has_one` | absent, not fabricated | | `test_nearest_repo_url_wins_over_a_more_distant_ancestor` | the nearest carrier wins — the only test that can tell nearest from farthest | | `test_a_blank_repo_url_does_not_mask_a_real_one_further_up` | a blank value is walked past, not published | -| `test_legacy_single_sbom_carries_the_element_path` | `analyze_component_section` publishes the path and omits `group` | + +Transitive mode: + +| Test | Asserts | +|---|---| +| `test_transitive_internal_components_carry_their_location` | the inlined `repoB` component carries `group` and `elementPath`, and keeps `softagram:internal` | +| `test_inlined_mirror_is_told_apart_from_its_host_by_its_published_location` | a component and its host share a name; location and the vcs URL separate them | `repo_url` is added to `repoA` in the existing multi fixture. This is safe: the one existing test -asserting on those `externalReferences` filters `type == 'bom'` first. +asserting on those `externalReferences` filters `type == 'bom'` first. `repoB` deliberately keeps +none, so it can serve as the absence case. A new fixture `modelfile_for_sbom_mirrored_tests.xml` carries one repository name under two -groups — the case the feature exists to disambiguate, which no current fixture covers: +groups — the case the feature exists to disambiguate, which no existing fixture covers. `GroupA` +also carries a `repo_url` of its own, which is what makes the nearest-wins rule testable: it is +the only ancestor chain in the suite with two carriers on it. | Test | Asserts | |---|---| -| `test_mirrored_repositories_are_distinguished_by_location` | same `name`, distinct `group`, `elementPath` and `serialNumber` | +| `test_mirrored_repositories_are_distinguished_by_their_location` | same `name`, distinct `group`, `elementPath` and `serialNumber` | +| `test_mirrored_repositories_carry_their_own_distinct_repository_urls` | each mirror publishes its own remote, not its neighbour's | ### Verification against a real model diff --git a/src/sgraph/converters/sbom_cyclonedx_generator.py b/src/sgraph/converters/sbom_cyclonedx_generator.py index 727a994..2467b9c 100644 --- a/src/sgraph/converters/sbom_cyclonedx_generator.py +++ b/src/sgraph/converters/sbom_cyclonedx_generator.py @@ -603,6 +603,13 @@ def elem_as_bom_data(elem, other_externals_by_name, external_root, noisy=False): } ] + These components describe 3rd-party packages, not model elements, so they deliberately get + neither the 'group'/'softagram:elementPath' pair nor a vcs reference from + _add_element_location / _add_vcs_reference. Their identity is the purl; their position in + the External subtree is an artefact of how the analyzer records dependencies, not a location + a consumer should navigate by. Locating them would also misattribute them: the ancestor walk + for a repo_url would climb out of External to the analyzed organisation's own repository. + :param elem: element :param other_externals_by_name: dict of external elements by name :param external_root: @@ -809,6 +816,13 @@ def _add_vcs_reference(component, elem): Call this only for components describing INTERNAL model elements. Were it run against a 3rd-party component in the External subtree, the walk would climb out to the estate root and attribute that package to the analyzed organisation's own repository. + + The walk deliberately reaches past the repository, up to the group and the estate root: it + has no reliable way to recognise a repository boundary, since repo elements are not required + to carry a 'type' attribute. The consequence is worth knowing. A repository that genuinely + has no remote, sitting under a group that does, reports the GROUP's url as its own. The + convention places repo_url on repo elements (docs/graph-conventions.md), which keeps this + rare, but it is inheritance by proximity, not proof of ownership. """ ancestor = elem while ancestor is not None: @@ -892,10 +906,12 @@ def analyze_component_section(elem, sbom): 'url': f'https://UNKNOWN-REPOSITORY_LOCATION/{repo.name}', 'type': 'vcs' }) - # Location only: this path builds its own vcs references from the element's children just - # above, including a fabricated placeholder when a child has no repo_url. That placeholder - # is a defect, but removing it changes output for existing consumers and is a separate - # decision — so this change neither adopts _add_vcs_reference here nor touches it. + # Location only, deliberately: this path builds its own vcs references just above, from the + # element's typed CHILDREN rather than its ancestors, and fabricates a placeholder url for a + # typed child that has no repo_url. That placeholder is a defect, but removing it changes + # output for existing consumers and is a separate decision, so _add_vcs_reference is neither + # adopted here nor allowed to touch it. Note the cardinality differs as a result: this path + # can emit one vcs reference per child, where _add_vcs_reference emits at most one. _add_element_location(c, elem) sbom.metadata_component = c