feat: support package+license specific exclusions#1047
feat: support package+license specific exclusions#1047erikburt wants to merge 1 commit intoactions:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Adds support for configuring allow-dependencies-licenses entries that target a specific license via a ?license=... PURL qualifier, while preserving the previous wildcard behavior when no license qualifier is provided.
Changes:
- Extend PURL parsing to extract a
licensequalifier. - Update license-change filtering to support both wildcard and license-qualified allow-list entries.
- Update docs and tests to cover license-qualified exclusions.
Reviewed changes
Copilot reviewed 6 out of 8 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/purl.ts | Parses license from PURL qualifiers to enable license-qualified filtering. |
| src/main.ts | Renames the config key passed into license validation to allowedDependenciesLicenses. |
| src/licenses.ts | Refactors and splits filtering to apply wildcard exclusions pre-license-fetch and license-qualified exclusions post-license-determination. |
| docs/examples.md | Documents how to exclude dependency+license combinations using ?license= qualifier. |
| tests/purl.test.ts | Adds tests verifying license qualifier parsing. |
| tests/licenses.test.ts | Adds tests for wildcard vs license-qualified allow dependency filtering behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| function parseLicenseQualifier(purl: string): string | null { | ||
| // Qualifiers are between '?' and '#', if present. | ||
| const qIndex = purl.indexOf('?') | ||
| if (qIndex !== -1) { | ||
| const hashIndex = purl.indexOf('#', qIndex + 1) | ||
| const query = purl.slice( | ||
| qIndex + 1, | ||
| hashIndex === -1 ? undefined : hashIndex | ||
| ) |
There was a problem hiding this comment.
The comment in parseLicenseQualifier states qualifiers are between ? and #, but the implementation will also parse license when ? appears after # (because it slices from ? to end when no later # exists). Since the tests explicitly cover ...#subpath?license=..., please update the comment to match the supported behavior (or enforce the stated ordering if that’s the intended contract).
| const preFilteredChanges = changes.filter(change => | ||
| filterLicenseChange(change, filtersWithNoLicenseQualifier) | ||
| ) |
There was a problem hiding this comment.
filterLicenseChange reparses change.package_url on every call and then linearly scans the full allow-list. Since this runs once per change (and potentially again for groupedChanges.licensed), it can become O(changes × allowedList) with repeated parsing. Consider pre-parsing changes once (e.g., attach parsed PURLs in a temporary structure) and/or building an index (e.g., Map keyed by type|namespace|name with a set of allowed licenses and/or a wildcard flag) so each change check is O(1) or close to it.
| const changeAsPackageURL = parsePURL(change.package_url) | ||
|
|
||
| for (const allowedDep of allowedDependenciesLicenses) { | ||
| if ( | ||
| allowedDep.type !== changeAsPackageURL.type || | ||
| allowedDep.namespace !== changeAsPackageURL.namespace || | ||
| allowedDep.name !== changeAsPackageURL.name | ||
| ) { | ||
| continue | ||
| } |
There was a problem hiding this comment.
filterLicenseChange reparses change.package_url on every call and then linearly scans the full allow-list. Since this runs once per change (and potentially again for groupedChanges.licensed), it can become O(changes × allowedList) with repeated parsing. Consider pre-parsing changes once (e.g., attach parsed PURLs in a temporary structure) and/or building an index (e.g., Map keyed by type|namespace|name with a set of allowed licenses and/or a wildcard flag) so each change check is O(1) or close to it.
| if ( | ||
| change.package_url.length === 0 || | ||
| allowedDependenciesLicenses === undefined || | ||
| allowedDependenciesLicenses?.length === 0 |
There was a problem hiding this comment.
The condition mixes an explicit === undefined check with optional chaining (allowedDependenciesLicenses?.length). Since allowedDependenciesLicenses === undefined is already covered, the optional-chain is redundant and slightly harder to read. Consider simplifying to a single nullish/length guard (e.g., !allowedDependenciesLicenses || allowedDependenciesLicenses.length === 0).
| allowedDependenciesLicenses?.length === 0 | |
| allowedDependenciesLicenses.length === 0 |
Adds support so
allowedDependenciesLicensescan be configured for specific licenses. This addresses #1046.It does this by adding support for a
licensePURL qualifier on the list of allowed dependencies licenses. This maintains backwards compatibility when thelicensequalifier is not included.See my changes to
example.mdfor more information.Changes
licensequalifierlicenses.tslogic to support filtering changes based onlicenselicenses.ts
filterLicenseChange, rather than it being inlined intogroupChangesgetInvalidLicenseChangesfunctiongroupChangesfunction used to prefilter the changes with the exclusion list. It was able to blanket filter here as it didn't matter if a dependency's license had been determined or not. And filtering before the grouping means less licenses to pull.allowedDependenciesLicensesto match input/config), into 2 separate lists. 1: Those without?license=...PURL qualifiers, and those with it.groupChangesas normal, but apply exclusions that containlicensequalifiers, after licenses have been determinedNotes
This does work but if the design is not acceptable, I'm happy to modify it.