From cefb29c8b76c17f8471030ca0236a4b66cd577f7 Mon Sep 17 00:00:00 2001 From: Ville Laitila Date: Mon, 3 Aug 2026 12:02:50 +0300 Subject: [PATCH 1/5] test: add maven coordinate fixture and tests (red by design, TDD P0) --- ...lfile_for_sbom_maven_coordinates_tests.xml | 56 ++++++ .../sbom_cyclonedx_generator_test.py | 187 ++++++++++++++++++ 2 files changed, 243 insertions(+) create mode 100644 tests/converters/modelfile_for_sbom_maven_coordinates_tests.xml diff --git a/tests/converters/modelfile_for_sbom_maven_coordinates_tests.xml b/tests/converters/modelfile_for_sbom_maven_coordinates_tests.xml new file mode 100644 index 0000000..a17424d --- /dev/null +++ b/tests/converters/modelfile_for_sbom_maven_coordinates_tests.xml @@ -0,0 +1,56 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/tests/converters/sbom_cyclonedx_generator_test.py b/tests/converters/sbom_cyclonedx_generator_test.py index a359827..ecda9af 100644 --- a/tests/converters/sbom_cyclonedx_generator_test.py +++ b/tests/converters/sbom_cyclonedx_generator_test.py @@ -302,3 +302,190 @@ def test_bom_ref_still_returns_only_the_purl_string(): '/ExampleOrg/External/Unknown_Binary_Files/Ionic.Zip/Ionic.Zip of version 1.9.1.8') assert elem is not None assert sbom_cyclonedx_generator.bom_ref(elem, '1.9.1.8') == 'pkg:nuget/Ionic.Zip@1.9.1.8' + + +# --- maven coordinate tests --- + +MAVEN_COORDINATES_MODEL = 'converters/modelfile_for_sbom_maven_coordinates_tests.xml' + + +def get_maven_coordinate_components(): + """Generate the maven-coordinate SBOM and index its components by component name.""" + model, _ = get_model_and_model_api(MAVEN_COORDINATES_MODEL) + sbom = sbom_cyclonedx_generator.generate_from_sgraph(model) + return {component['name']: component for component in sbom['components']} + + +def test_maven_coordinates_supply_the_required_groupid_namespace(): + """The maven type requires a namespace, and the coordinates to build one were already there. + + Pinned as an exact set rather than per fixture: a missing purl and an unexpected extra one + both fail, and an empty result cannot pass. + """ + purls = set() + for model_file in ('converters/modelfile_for_sbom_tests.xml', + 'converters/modelfile_for_sbom_multi_tests.xml', + BINARY_REFS_MODEL): + model, _ = get_model_and_model_api(model_file) + sbom = sbom_cyclonedx_generator.generate_from_sgraph(model) + purls.update(c['purl'] for c in sbom['components'] if c['purl'].startswith('pkg:maven/')) + assert purls == { + 'pkg:maven/aopalliance/aopalliance@1.0', + 'pkg:maven/org.apache.commons/commons-lang3@3.12.0', + 'pkg:maven/org.example.sample/sample-lib@2.4.1', + } + + +def test_multi_sbom_dependson_carries_a_well_formed_maven_reference(): + """A dependsOn entry is a bom-ref, and the maven one used to carry a raw space into it.""" + model, _ = get_model_and_model_api('converters/modelfile_for_sbom_multi_tests.xml') + result = generate_multi_from_sgraph(model, level=3) + repo_b_sbom = next(s for s in result if s['metadata']['component']['name'] == 'repoB') + repo_b_ref = repo_b_sbom['metadata']['component']['bom-ref'] + depends_on = next(d for d in repo_b_sbom['dependencies'] + if d['ref'] == repo_b_ref)['dependsOn'] + assert 'pkg:maven/org.apache.commons/commons-lang3@3.12.0' in depends_on + assert not any(' ' in reference for reference in depends_on) + + +def test_partial_maven_coordinates_do_not_emit_half_an_identity(): + """Only one coordinate present must not produce a half-identity. With both coordinates absent, + requiring both and requiring either behave identically, so no both-absent fixture can + separate them. This element can, and under the relaxed rule it emits a namespace with an + empty name. + """ + component = get_maven_coordinate_components()['partial-lib'] + assert component['purl'] == 'pkg:generic/partial-lib@1.0' + assert purl_type_resolution(component) == 'maven coordinates unavailable' + + +def test_maven_element_without_coordinates_takes_the_residual_and_is_not_inferred(): + """The Maven branch resolves its own fallback rather than falling through to inference. + + The fixture references this element from a .dll, which would vote nuget if the + coordinate-less case reached infer_pkgtype_from_referencing_files. Both assertions below + therefore separate an honest residual from a guess, which the type alone would not. + """ + component = get_maven_coordinate_components()['coordinateless-lib'] + assert component['purl'] == 'pkg:generic/coordinateless-lib@2.0' + assert purl_type_resolution(component) == 'maven coordinates unavailable' + + +def test_a_purl_legal_character_that_is_not_a_maven_id_takes_the_residual(): + """A colon is legal in a purl component and can never be a Maven id. + + A purl built from it would be canonical, would pass a conformance checker, and would match + nothing. That is why the guard is the Maven id charset and not the purl charset. + """ + component = get_maven_coordinate_components()['colon-lib'] + assert component['purl'] == 'pkg:generic/colon-lib@3.0' + assert purl_type_resolution(component) == 'maven coordinates unavailable' + + +def test_the_residual_value_stays_distinct_from_an_unresolved_ecosystem(): + """A Maven element with no usable coordinates has a known ecosystem and an unusable name. + + Reusing 'ecosystem unresolved' here would mark as ecosystem-unknown a component whose + ecosystem is known, blunting the discriminator the previous release told consumers to grep. + """ + components = get_maven_coordinate_components() + residuals = {purl_type_resolution(components[name]) + for name in ('partial-lib', 'coordinateless-lib', 'colon-lib')} + assert residuals == {'maven coordinates unavailable'} + assert 'ecosystem unresolved' not in residuals + + +def test_maven_coordinate_case_is_preserved_in_both_components(): + """The maven type is case-sensitive in namespace and name, so neither may be lowercased. + + Asserts its own premise: the Maven bucket in this fixture sits under a JVM layer, as analyzer + output does, and the branch matches the package element's parent so the extra layer makes no + difference. Nothing in the output reveals the layer, so without the premise assertion, + flattening the fixture would silently retire the only tracked evidence of that. + """ + model, _ = get_model_and_model_api(MAVEN_COORDINATES_MODEL) + elem = model.findElementFromPath( + '/ExampleOrg/External/JVM/Maven/Org.Example.Mixed Mixed-Lib of version 4.0') + assert elem is not None + component = get_maven_coordinate_components()['Org.Example.Mixed Mixed-Lib'] + assert component['purl'] == 'pkg:maven/Org.Example.Mixed/Mixed-Lib@4.0' + assert purl_type_resolution(component) is None + + +def test_an_unresolved_version_yields_a_versionless_purl(): + """A build-property expression names no published artifact, so the version is omitted. + + Omission is not a workaround: purl treats the version as optional, so the result is canonical + and matches at package level, where the raw expression could only ever match nothing. The raw + expression stays in the component's version field, so it is disclosed rather than dropped. + """ + component = get_maven_coordinate_components()['org.example.unresolved unresolved-lib'] + assert component['purl'] == 'pkg:maven/org.example.unresolved/unresolved-lib' + assert component['version'] == '${project.version}' + assert purl_type_resolution(component) is None + + +def test_maven_coordinate_guard_accepts_real_coordinates(): + """Anti-vacuity for the guard: it must admit ordinary and uppercase coordinates.""" + for value in ('org.apache.commons', 'commons-lang3', 'aopalliance', 'HTTPClient'): + assert sbom_cyclonedx_generator.is_maven_coordinate(value), value + + +def test_maven_coordinate_guard_does_more_than_reject_whitespace(): + """Control against simplifying the guard to a space check. + + Only two of the values below contain whitespace at all, asserted rather than claimed, so a + guard reduced to a whitespace test would accept the other eight. Two of those eight, + 'org.example:lib' and 'org.example~lib', purl would leave unencoded: they are what + distinguishes a Maven-charset guard from a purl-charset one. '.' and '..' are rejected by an + explicit exclusion, since the pattern alone matches both. + """ + not_maven_ids = ('org.example sample-lib', 'org.example\tlib', '', '.', '..', + 'org/example', 'org.example:lib', 'org.example~lib', + '${project.version}', 'org.exämple') + for value in not_maven_ids: + assert not sbom_cyclonedx_generator.is_maven_coordinate(value), value + whitespace_bearing = [v for v in not_maven_ids if any(c.isspace() for c in v)] + assert len(whitespace_bearing) == 2 + + +def test_maven_coordinates_fixture_yields_five_distinct_components(): + """Anti-vacuity for the fixture, and the collapse guard for version omission. + + A lookup above raises when an element vanishes, but an element added, or two bom-refs + collapsed into one when a version is omitted, would otherwise go unnoticed. + """ + model, _ = get_model_and_model_api(MAVEN_COORDINATES_MODEL) + sbom = sbom_cyclonedx_generator.generate_from_sgraph(model) + assert len(sbom['components']) == 5 + assert len({component['bom-ref'] for component in sbom['components']}) == 5 + + +def test_a_partly_resolved_version_keeps_its_version(): + """Only a whole build-property expression is dropped, not a version that merely contains one. + + Nothing else can fail if UNRESOLVED_VERSION is widened from fullmatch to a substring search: + no fixture carries a partly resolved version, so every other assertion stays green while + '1.0-${suffix}' silently loses the resolved part it should have kept. + """ + model = SGraph(SElement(None, '')) + elem = model.createOrGetElementFromPath('/Proj/External/Maven/lib') + elem.attrs.update({'groupId': 'org.example', 'artifactId': 'lib'}) + assert sbom_cyclonedx_generator.maven_purl(elem, '1.0-${suffix}') == \ + 'pkg:maven/org.example/lib@1.0-${suffix}' + assert sbom_cyclonedx_generator.maven_purl(elem, '${suffix}') == 'pkg:maven/org.example/lib' + + +def test_a_caret_prefixed_version_is_normalised_as_on_every_other_branch(): + """The maven path keeps the version handling the shared tail gives every other type. + + purl_for strips a leading caret before splicing the version in. The maven path returns + early, so it has to apply that itself, and no fixture carries a caret version: dropping the + call changes no other assertion in this file. + """ + maven_bucket = SElement(None, 'Maven') + elem = SElement(maven_bucket, 'org.example example-lib of version 1.0') + elem.attrs.update(groupId='org.example', artifactId='example-lib') + purl, properties = sbom_cyclonedx_generator.purl_for(elem, '^1.0') + assert purl == 'pkg:maven/org.example/example-lib@1.0' + assert properties == [] From b48762b6dedfca7505934efadfff19ab2fd2d150 Mon Sep 17 00:00:00 2001 From: Ville Laitila Date: Mon, 3 Aug 2026 12:15:33 +0300 Subject: [PATCH 2/5] SBOM: build maven purls from the model's Maven coordinates --- .../converters/sbom_cyclonedx_generator.py | 59 ++++++++++++++++++- .../sbom_cyclonedx_generator_test.py | 8 +-- 2 files changed, 62 insertions(+), 5 deletions(-) diff --git a/src/sgraph/converters/sbom_cyclonedx_generator.py b/src/sgraph/converters/sbom_cyclonedx_generator.py index 4fc05a5..adc2660 100644 --- a/src/sgraph/converters/sbom_cyclonedx_generator.py +++ b/src/sgraph/converters/sbom_cyclonedx_generator.py @@ -95,6 +95,30 @@ def parents_parent_or_parent_name_equals(elem, name): PURL_TYPE_SOURCE_PROPERTY = 'purlTypeResolution' +# Maven's model validator restricts groupId and artifactId to [A-Za-z0-9._-] at ERROR severity +# for every POM, and that set is a strict subset of the characters purl leaves unencoded. So a +# coordinate that passes this guard is spliced into the purl verbatim and needs no encoding +# step, which is what makes deferring encoding coherent rather than a shortcut. +MAVEN_COORDINATE = re.compile(r'^[A-Za-z0-9._-]+$') + +# A Maven version the analyzer captured before the build resolved it. Matched with fullmatch, +# deliberately: a version that merely contains an expression, such as '1.0-${suffix}', is partly +# known, and dropping the known part would change which components exist under a rule nobody has +# measured. Widening this to a substring search is the tempting simplification to refuse. +UNRESOLVED_VERSION = re.compile(r'\$\{.+\}') + + +def is_maven_coordinate(value): + """Whether a value can be a Maven groupId or artifactId, and so a purl component. + + The dot check is not redundant with the pattern: '.' and '..' match it and are not ids. + Maven's local repository layout uses coordinate ids verbatim as directory names, so + accepting them would splice a path-traversal segment into the position where a namespace + belongs. No real coordinate is a dot, so removing this clause leaves every test drawn from + coordinate data green. + """ + return bool(MAVEN_COORDINATE.match(value)) and value not in {'.', '..'} + def infer_pkgtype_from_referencing_files(elem): """Infer a purl type from the extensions of the files referencing this element. @@ -126,6 +150,31 @@ def infer_pkgtype_from_referencing_files(elem): return winner, sorted(extensions_by_pkgtype[winner]) +def maven_purl(elem, version): + """Build the maven purl from an element's coordinates, or None when it has none usable. + + The maven type definition makes the namespace required and names groupId as its native + name, so a single-segment pkg:maven/ is not merely unencoded, it is unmatchable: + Maven Central identity is groupId:artifactId. Both coordinates are read straight off the + element, so this is a projection of what the model already holds, not an inference about it. + + None covers absent, partial and charset-rejected coordinates alike: they take the same + residual, so the caller does not need to tell them apart. + + An unresolved version yields a versionless purl rather than one carrying the expression. A + purl version must be percent-encoded, so the expression would be either non-canonical raw or + canonical-but-unmatchable encoded; omitting it yields a purl that is canonical and still + matches at package level. + """ + group_id = elem.attrs.get('groupId', '') + artifact_id = elem.attrs.get('artifactId', '') + if not (is_maven_coordinate(group_id) and is_maven_coordinate(artifact_id)): + return None + if UNRESOLVED_VERSION.fullmatch(version): + return f'pkg:maven/{group_id}/{artifact_id}' + return f'pkg:maven/{group_id}/{artifact_id}@{version}' + + def purl_for(elem, v): """Build the purl of an element and report how its package type was resolved. @@ -148,8 +197,16 @@ def purl_for(elem, v): pkgtype = 'pypi' # ?? elif elem.parent.name == 'Go': pkgtype = 'golang' + # Coordinates read from attributes are not a guess, so no resolution property is appended. elif elem.parent.name == 'Maven': - pkgtype = 'maven' + maven = maven_purl(elem, v.lstrip('^')) + if maven is not None: + return maven, properties + pkgtype = FALLBACK_PURL_TYPE + properties.append({ + 'name': PURL_TYPE_SOURCE_PROPERTY, + 'value': 'maven coordinates unavailable' + }) elif incoming_deps(elem, ['csproj', 'vbproj'], ['assembly_ref']) or parents_parent_or_parent_name_equals( elem, 'Assemblies'): diff --git a/tests/converters/sbom_cyclonedx_generator_test.py b/tests/converters/sbom_cyclonedx_generator_test.py index ecda9af..0d0463c 100644 --- a/tests/converters/sbom_cyclonedx_generator_test.py +++ b/tests/converters/sbom_cyclonedx_generator_test.py @@ -259,10 +259,10 @@ def test_types_read_from_attributes_or_ancestors_carry_no_provenance(): assert components['ExampleOrg.Common']['purl'] == 'pkg:nuget/ExampleOrg.Common@1.4.2' assert purl_type_resolution(components['ExampleOrg.Common']) is None maven_component = components['org.example.sample sample-lib'] - # Characterization of known-nonconforming output: the name keeps its literal space (names - # are emitted unencoded — see the type-only caveat in the generator). A name-encoding fix - # should update this expected string, not relax the assertion. - assert maven_component['purl'] == 'pkg:maven/org.example.sample sample-lib@2.4.1' + # This component is still looked up by a name that carries the space; only the purl stopped + # carrying it. Maven ids cannot contain a space, so encoding it as %20 here would have been + # spec-valid and still matched nothing. + assert maven_component['purl'] == 'pkg:maven/org.example.sample/sample-lib@2.4.1' assert purl_type_resolution(maven_component) is None From de938dc572f8a7421e25aafcbe301689fb490eee Mon Sep 17 00:00:00 2001 From: Ville Laitila Date: Mon, 3 Aug 2026 12:20:02 +0300 Subject: [PATCH 3/5] SBOM: scope the type-only purl caveats now that maven purls carry a namespace --- src/sgraph/converters/sbom_cyclonedx_generator.py | 2 +- tests/converters/sbom_cyclonedx_generator_test.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/sgraph/converters/sbom_cyclonedx_generator.py b/src/sgraph/converters/sbom_cyclonedx_generator.py index adc2660..8197950 100644 --- a/src/sgraph/converters/sbom_cyclonedx_generator.py +++ b/src/sgraph/converters/sbom_cyclonedx_generator.py @@ -82,7 +82,7 @@ def parents_parent_or_parent_name_equals(elem, name): # Same rule, opposite outcomes, because the type definitions differ. pypi and gem also prohibit a # namespace, so whl/egg/gem are safe to infer. # -# This fixes the purl TYPE only. Package names are still emitted unencoded throughout this +# This table fixes the purl TYPE only. Package names are still emitted unencoded throughout this # module, so a spec-valid type does not by itself make a purl spec-conforming. PURL_TYPE_BY_REFERENCING_EXTENSION = { 'dll': 'nuget', diff --git a/tests/converters/sbom_cyclonedx_generator_test.py b/tests/converters/sbom_cyclonedx_generator_test.py index 0d0463c..61e0131 100644 --- a/tests/converters/sbom_cyclonedx_generator_test.py +++ b/tests/converters/sbom_cyclonedx_generator_test.py @@ -116,8 +116,8 @@ def test_multi_sbom_internal_dependencies(): BINARY_REFS_MODEL = 'converters/modelfile_for_sbom_binary_refs_tests.xml' # The purl spec requires a type to start with a letter and to hold only [a-z0-9.-] in its -# canonical form. Only the type is anchored on purpose: name encoding is a separate, pre-existing -# concern (existing fixtures legitimately produce maven names that contain a space). +# canonical form. Only the type is anchored here: this pattern predates maven coordinates and +# still says nothing about namespace, name or version, which other assertions cover. PURL_TYPE_PATTERN = re.compile(r'^pkg:[a-z][a-z0-9.-]*/') From 8585386eaa329ebfe8d597e00258edc4107c6d04 Mon Sep 17 00:00:00 2001 From: Ville Laitila Date: Mon, 3 Aug 2026 12:50:56 +0300 Subject: [PATCH 4/5] SBOM: reject a trailing newline in maven coordinates --- src/sgraph/converters/sbom_cyclonedx_generator.py | 2 +- tests/converters/sbom_cyclonedx_generator_test.py | 11 +++++++---- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/src/sgraph/converters/sbom_cyclonedx_generator.py b/src/sgraph/converters/sbom_cyclonedx_generator.py index 8197950..1ac2600 100644 --- a/src/sgraph/converters/sbom_cyclonedx_generator.py +++ b/src/sgraph/converters/sbom_cyclonedx_generator.py @@ -117,7 +117,7 @@ def is_maven_coordinate(value): belongs. No real coordinate is a dot, so removing this clause leaves every test drawn from coordinate data green. """ - return bool(MAVEN_COORDINATE.match(value)) and value not in {'.', '..'} + return bool(MAVEN_COORDINATE.fullmatch(value)) and value not in {'.', '..'} def infer_pkgtype_from_referencing_files(elem): diff --git a/tests/converters/sbom_cyclonedx_generator_test.py b/tests/converters/sbom_cyclonedx_generator_test.py index 61e0131..a295000 100644 --- a/tests/converters/sbom_cyclonedx_generator_test.py +++ b/tests/converters/sbom_cyclonedx_generator_test.py @@ -434,19 +434,22 @@ def test_maven_coordinate_guard_accepts_real_coordinates(): def test_maven_coordinate_guard_does_more_than_reject_whitespace(): """Control against simplifying the guard to a space check. - Only two of the values below contain whitespace at all, asserted rather than claimed, so a + Only three of the values below contain whitespace at all, asserted rather than claimed, so a guard reduced to a whitespace test would accept the other eight. Two of those eight, 'org.example:lib' and 'org.example~lib', purl would leave unencoded: they are what distinguishes a Maven-charset guard from a purl-charset one. '.' and '..' are rejected by an - explicit exclusion, since the pattern alone matches both. + explicit exclusion, since the pattern alone matches both. The trailing-newline value is why + the pattern is applied with fullmatch: '$' also matches before a final newline, so under + match() that one value would be accepted and emit a purl with a line break inside the + namespace. """ not_maven_ids = ('org.example sample-lib', 'org.example\tlib', '', '.', '..', 'org/example', 'org.example:lib', 'org.example~lib', - '${project.version}', 'org.exämple') + '${project.version}', 'org.exämple', 'probe.group\n') for value in not_maven_ids: assert not sbom_cyclonedx_generator.is_maven_coordinate(value), value whitespace_bearing = [v for v in not_maven_ids if any(c.isspace() for c in v)] - assert len(whitespace_bearing) == 2 + assert len(whitespace_bearing) == 3 def test_maven_coordinates_fixture_yields_five_distinct_components(): From db6196f14298314b69403ddcb6c06ddd5ec977d3 Mon Sep 17 00:00:00 2001 From: Ville Laitila Date: Mon, 3 Aug 2026 12:55:17 +0300 Subject: [PATCH 5/5] SBOM: state the trailing-newline guard rationale in the test docstring --- tests/converters/sbom_cyclonedx_generator_test.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/converters/sbom_cyclonedx_generator_test.py b/tests/converters/sbom_cyclonedx_generator_test.py index a295000..cbf112f 100644 --- a/tests/converters/sbom_cyclonedx_generator_test.py +++ b/tests/converters/sbom_cyclonedx_generator_test.py @@ -438,10 +438,10 @@ def test_maven_coordinate_guard_does_more_than_reject_whitespace(): guard reduced to a whitespace test would accept the other eight. Two of those eight, 'org.example:lib' and 'org.example~lib', purl would leave unencoded: they are what distinguishes a Maven-charset guard from a purl-charset one. '.' and '..' are rejected by an - explicit exclusion, since the pattern alone matches both. The trailing-newline value is why - the pattern is applied with fullmatch: '$' also matches before a final newline, so under - match() that one value would be accepted and emit a purl with a line break inside the - namespace. + explicit exclusion, since the pattern alone matches both. The trailing-newline value is there + for a different reason from the rest: the pattern's '$' also matches just before a final + newline, so match() accepts a coordinate that ends in one and would emit a purl with a line + break inside the namespace. Only fullmatch's whole-string requirement refuses it. """ not_maven_ids = ('org.example sample-lib', 'org.example\tlib', '', '.', '..', 'org/example', 'org.example:lib', 'org.example~lib',