Skip to content

fix: use PackageURL to canonicalize purls when matching deps.dev license response to SBOM purls#414

Merged
ruromero merged 2 commits into
guacsec:mainfrom
soul2zimate:incompatibleDependencies
Mar 18, 2026
Merged

fix: use PackageURL to canonicalize purls when matching deps.dev license response to SBOM purls#414
ruromero merged 2 commits into
guacsec:mainfrom
soul2zimate:incompatibleDependencies

Conversation

@soul2zimate

Copy link
Copy Markdown
Contributor

fixes: #413

@soul2zimate soul2zimate requested a review from ruromero March 17, 2026 09:29
@qodo-code-review

Copy link
Copy Markdown

Review Summary by Qodo

Fix purl qualifier mismatch in license response normalization

🐞 Bug fix 🧪 Tests

Grey Divider

Walkthroughs

Description
• Strip purl qualifiers when matching deps.dev responses to SBOM purls
• Fix incompatibleDependencies always being empty due to qualifier mismatch
• Store qualifier-free purls as map keys for consistent lookups
• Add three comprehensive tests for qualifier handling scenarios
Diagram
flowchart LR
  A["Backend Response<br/>with ?scope=compile"] -- "Strip qualifiers" --> B["Normalized Purl<br/>Base"]
  C["SBOM Purls<br/>without qualifiers"] -- "Strip qualifiers" --> B
  B -- "Compare & Match" --> D["Map with<br/>Qualifier-free Keys"]
Loading

Grey Divider

File Changes

1. src/license/licenses_api.js 🐞 Bug fix +4/-2

Strip qualifiers from purls before matching and storing

• Extract purl base by splitting on '?' to remove qualifiers
• Update matching logic to compare qualifier-free purl bases
• Store qualifier-stripped purl as map key instead of original
• Preserve license category and identifiers in map values

src/license/licenses_api.js


2. test/providers/license.test.js 🧪 Tests +50/-0

Add comprehensive tests for purl qualifier handling

• Add new test suite for normalizeLicensesResponse function
• Create mock backend response with ?scope=compile qualifiers
• Create mock SBOM purls without qualifiers
• Test matching of qualified backend purls to plain SBOM purls
• Test that map keys store qualifier-free purls
• Test that license categories are preserved correctly

test/providers/license.test.js


Grey Divider

Qodo Logo

@qodo-code-review

qodo-code-review Bot commented Mar 17, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (0) 📎 Requirement gaps (0)

Grey Divider


Remediation recommended

1. Quadratic purl matching 🐞 Bug ➹ Performance
Description
normalizeLicensesResponse now calls purls.some(...) for every backend package entry, making runtime
O(number_of_packages × number_of_purls) and repeatedly splitting strings. With large SBOMs/license
responses this can become a noticeable bottleneck during component analysis license checking.
Code

src/license/licenses_api.js[R67-70]

+			const purlBase = purl.split('?')[0];
+			const matched = purls.length === 0 || purls.some(p => p.split('?')[0] === purlBase);
+			if (matched) {
+				map.set(purlBase, { licenses: licenses.filter(Boolean), category });
Evidence
The implementation loops over every entry in providerResult.packages and performs a linear scan over
the purls array for each entry via purls.some(), with additional per-element string splitting.

src/license/licenses_api.js[58-72]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`normalizeLicensesResponse()` does a linear scan over `purls` for every backend package entry (`purls.some(...)`), resulting in O(N×M) runtime and repeated `split(&#x27;?&#x27;)` calls.

### Issue Context
This function runs as part of license checking after component analysis; for large SBOMs and license responses it can add avoidable overhead.

### Fix Focus Areas
- src/license/licenses_api.js[54-76]

### Suggested direction
- Precompute `const allowed = new Set(purls.map(p =&gt; p.split(&#x27;?&#x27;)[0]));`
- Replace `purls.some(...)` with `allowed.has(purlBase)` when `purls.length &gt; 0`.
- Optionally factor out a small helper to normalize purls consistently.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Advisory comments

2. Qualifier-stripped key change 🐞 Bug ✓ Correctness
Description
When normalizeLicensesResponse is called with an empty purls filter, it now stores results under the
qualifier-stripped key rather than the exact backend purl key. This makes lookups by the original
backend purl string (including qualifiers) impossible and changes the behavior of this exported API.
Code

src/license/licenses_api.js[R67-70]

+			const purlBase = purl.split('?')[0];
+			const matched = purls.length === 0 || purls.some(p => p.split('?')[0] === purlBase);
+			if (matched) {
+				map.set(purlBase, { licenses: licenses.filter(Boolean), category });
Evidence
The function is part of the library’s public exports, and it now sets Map keys to purlBase
(substring before '?'), not the original purl from the backend response. Therefore, if the backend
response key includes qualifiers, the Map will not contain that original key even when no filtering
is requested.

src/index.js[10-13]
src/license/licenses_api.js[54-76]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
When `purls` is omitted/empty, `normalizeLicensesResponse()` currently keys the returned Map by the qualifier-stripped `purlBase` instead of the exact backend response key. This changes observable behavior for callers that expect `map.has(originalBackendPurl)` to work.

### Issue Context
`normalizeLicensesResponse` is exported from the package entrypoint, so key-shape changes can be breaking for consumers.

### Fix Focus Areas
- src/license/licenses_api.js[54-76]

### Suggested direction
Option A (preserve old behavior when unfiltered):
- If `purls.length === 0`, keep `map.set(purl, ...)` (original key).
- If `purls.length &gt; 0`, do qualifier-agnostic matching but set the Map key to the caller’s purl (or to `purlBase`, as intended).

Option B (dual-key for compatibility):
- Always set both `map.set(purl, ...)` and `map.set(purlBase, ...)` (be cautious about overwrites).

Also update JSDoc to explicitly document whether qualifiers are stripped from keys.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

ⓘ The new review experience is currently in Beta. Learn more

Grey Divider

Qodo Logo

@soul2zimate soul2zimate requested a review from Strum355 March 17, 2026 09:29
@soul2zimate soul2zimate force-pushed the incompatibleDependencies branch from 2cf4fa3 to b26a6c1 Compare March 17, 2026 10:08
@soul2zimate

Copy link
Copy Markdown
Contributor Author

/review

@qodo-code-review

Copy link
Copy Markdown

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶

413 - Partially compliant

Compliant requirements:

  • Fix normalizeLicensesResponse so backend purls with qualifiers match SBOM purls without qualifiers
  • Add coverage that validates qualifier-stripping behavior and resulting map keys/categories

Non-compliant requirements:

  • Provide a way to reproduce/verify via CLI run against the example app and confirm incompatibleDependencies is populated

Requires further human verification:

  • Run the CLI flow from the ticket reproduction steps against the example repo and confirm incompatibleDependencies is no longer empty
⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 PR contains tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Edge Case

Qualifier stripping uses split('?')[0], which assumes ? is the only delimiter and that everything after it should be ignored. Validate this is correct for all purl variants you expect (e.g., qualifiers ordering, encoded ?, or other canonicalization needs) and consider using a purl parser/canonicalizer if available in the project.

const allowed = purls.length > 0 ? new Set(purls.map(p => p.split('?')[0])) : null;

for (const providerResult of data) {
	const packages = providerResult?.packages;
	if (!packages || typeof packages !== 'object') {continue;}
	for (const [purl, pkgLicense] of Object.entries(packages)) {
		const concluded = pkgLicense?.concluded;
		const identifiers = Array.isArray(concluded?.identifiers) ? concluded.identifiers : [];
		const expression = concluded?.expression;
		const licenses = identifiers.length > 0 ? identifiers : (expression ? [expression] : []);
		const category = concluded?.category; // PERMISSIVE | WEAK_COPYLEFT | STRONG_COPYLEFT | UNKNOWN
		if (allowed === null) {
			map.set(purl, { licenses: licenses.filter(Boolean), category });
		} else {
			const purlBase = purl.split('?')[0];
			if (allowed.has(purlBase)) {
				map.set(purlBase, { licenses: licenses.filter(Boolean), category });
			}
Key Collision

When allowed is provided, multiple backend entries that differ only by qualifiers will collapse into the same base purl key, and the last one iterated will win. Confirm this overwrite behavior is acceptable (or define merge/precedence rules) in cases where different qualifiers yield different license results.

if (allowed === null) {
	map.set(purl, { licenses: licenses.filter(Boolean), category });
} else {
	const purlBase = purl.split('?')[0];
	if (allowed.has(purlBase)) {
		map.set(purlBase, { licenses: licenses.filter(Boolean), category });
	}
Test Coverage

Current tests assert map.size and key normalization, but they don't validate that the stored licenses array values are correct after filtering, or that the function still behaves correctly when purls is empty (i.e., preserves original backend keys including qualifiers). Consider adding at least one assertion for licenses content and one test for the purls.length === 0 path.

suite('normalizeLicensesResponse', () => {
	const lgpl = { id: 'LGPL-2.1', name: 'GNU Lesser General Public License v2.1 only', category: 'WEAK_COPYLEFT' }
	const apache = { id: 'Apache-2.0', name: 'Apache License 2.0', category: 'PERMISSIVE' }

	const backendResponse = [
		{
			status: { ok: true, name: 'deps.dev' },
			packages: {
				// backend returns purls with ?scope=compile qualifier
				'pkg:maven/org.mariadb.jdbc/mariadb-java-client@3.1.4?scope=compile': {
					concluded: { identifiers: [lgpl], expression: 'LGPL-2.1', category: 'WEAK_COPYLEFT' }
				},
				'pkg:maven/javassist/javassist@3.12.1.GA?scope=compile': {
					concluded: { identifiers: [lgpl], expression: 'LGPL-2.1', category: 'WEAK_COPYLEFT' }
				},
				'pkg:maven/commons-collections/commons-collections@3.2.1': {
					concluded: { identifiers: [apache], expression: 'Apache-2.0', category: 'PERMISSIVE' }
				}
			}
		}
	]

	// SBOM purls have no qualifier
	const sbomPurls = [
		'pkg:maven/org.mariadb.jdbc/mariadb-java-client@3.1.4',
		'pkg:maven/javassist/javassist@3.12.1.GA',
		'pkg:maven/commons-collections/commons-collections@3.2.1'
	]

	test('matches backend purls with ?scope=compile qualifier against plain SBOM purls', () => {
		const map = normalizeLicensesResponse(backendResponse, sbomPurls)
		expect(map.size).to.equal(3)
	})

	test('stores purl without qualifier as map key', () => {
		const map = normalizeLicensesResponse(backendResponse, sbomPurls)
		expect(map.has('pkg:maven/org.mariadb.jdbc/mariadb-java-client@3.1.4')).to.be.true
		expect(map.has('pkg:maven/javassist/javassist@3.12.1.GA')).to.be.true
		expect(map.has('pkg:maven/org.mariadb.jdbc/mariadb-java-client@3.1.4?scope=compile')).to.be.false
	})

	test('preserves correct license category for qualifier-stripped purls', () => {
		const map = normalizeLicensesResponse(backendResponse, sbomPurls)
		expect(map.get('pkg:maven/org.mariadb.jdbc/mariadb-java-client@3.1.4').category).to.equal('WEAK_COPYLEFT')
		expect(map.get('pkg:maven/javassist/javassist@3.12.1.GA').category).to.equal('WEAK_COPYLEFT')
		expect(map.get('pkg:maven/commons-collections/commons-collections@3.2.1').category).to.equal('PERMISSIVE')
	})

@ruromero ruromero left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer to use the PackageURL to canonicalize the purls.
What if you define a function like this?

function getCorePurl(purl) {
	const parsed = PackageURL.fromString(purl);
	const corePurl = new PackageURL(
		parsed.type,
		parsed.namespace,
		parsed.name,
		parsed.version,
		null, // no qualifiers
		null  // no subpath
	);
	return corePurl.toString();
}

@soul2zimate soul2zimate force-pushed the incompatibleDependencies branch from b26a6c1 to ef6b443 Compare March 17, 2026 13:31
@soul2zimate soul2zimate changed the title fix: strip purl qualifiers when matching deps.dev license response to SBOM purls fix: use PackageURL to canonicalize purls when matching deps.dev license response to SBOM purls Mar 17, 2026
@soul2zimate soul2zimate changed the title fix: use PackageURL to canonicalize purls when matching deps.dev license response to SBOM purls fix: use PackageURL to canonicalize purls when matching deps.dev license response to SBOM purls Mar 17, 2026
@soul2zimate

Copy link
Copy Markdown
Contributor Author

Thanks for the review. fix has been updated.

@soul2zimate soul2zimate requested a review from ruromero March 17, 2026 13:35

@ruromero ruromero left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks.
Let me know what you think about my comments

Comment thread src/license/licenses_api.js Outdated
Comment thread src/license/licenses_api.js Outdated
…nse response to SBOM purls

The deps.dev backend returns package purls with qualifiers like ?scope=compile
(e.g. pkg:maven/org.mariadb.jdbc/mariadb-java-client@3.1.4?scope=compile),
while the SBOM generated by the client uses plain purls without qualifiers.
The strict purls.includes(purl) check in normalizeLicensesResponse never
matched qualifier-suffixed purls, causing incompatibleDependencies to always
be empty even when LGPL/copyleft dependencies were present.

Fix by using PackageURL.fromString() to parse purls and reconstructing them
without qualifiers or subpath via new PackageURL(..., null, null). This
produces canonical core purls for both the allowed set and the backend keys,
making the match resilient to any qualifiers the backend may return.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@soul2zimate soul2zimate force-pushed the incompatibleDependencies branch from ef6b443 to 7c7f9a9 Compare March 18, 2026 00:48
@soul2zimate soul2zimate requested a review from ruromero March 18, 2026 00:51
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@soul2zimate

Copy link
Copy Markdown
Contributor Author

added a second commit to remove unused getLicenseForSbom function after refactoring in #409

@ruromero ruromero left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fix

@ruromero ruromero merged commit cb4ae28 into guacsec:main Mar 18, 2026
4 checks passed
@soul2zimate soul2zimate deleted the incompatibleDependencies branch March 19, 2026 00:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

incompatibleDependencies always empty when backend returns purls with qualifiers

3 participants