Skip to content

fix(license): treat UNKNOWN-category dependencies as incompatible#548

Open
a-oren wants to merge 1 commit into
guacsec:mainfrom
a-oren:worktree-TC-4706
Open

fix(license): treat UNKNOWN-category dependencies as incompatible#548
a-oren wants to merge 1 commit into
guacsec:mainfrom
a-oren:worktree-TC-4706

Conversation

@a-oren
Copy link
Copy Markdown
Contributor

@a-oren a-oren commented Jun 4, 2026

Summary

  • Split the getCompatibility() UNKNOWN category check: dependencies with UNKNOWN license category are now flagged as incompatible when the project has a known category (PERMISSIVE, WEAK_COPYLEFT, STRONG_COPYLEFT)
  • Added a distinct reason message for UNKNOWN-category dependencies: "License not recognized as a standard SPDX identifier. Manual review recommended to verify compatibility."
  • Project-UNKNOWN still returns 'unknown' to avoid false positives
  • Added 9 new tests covering all UNKNOWN category combinations

Implements TC-4706

Test plan

  • getCompatibility('PERMISSIVE', 'UNKNOWN') returns 'incompatible'
  • getCompatibility('WEAK_COPYLEFT', 'UNKNOWN') returns 'incompatible'
  • getCompatibility('STRONG_COPYLEFT', 'UNKNOWN') returns 'incompatible'
  • getCompatibility('UNKNOWN', 'PERMISSIVE') returns 'unknown'
  • getCompatibility('UNKNOWN', 'UNKNOWN') returns 'unknown'
  • getCompatibility(null, 'UNKNOWN') returns 'unknown'
  • Existing compatibility checks unchanged
  • All 463 existing tests still pass

Summary by Sourcery

Tighten license compatibility handling for UNKNOWN-category dependencies and surface a clearer incompatibility reason during license checks.

New Features:

  • Treat dependencies with UNKNOWN license category as incompatible when the project license category is known.
  • Provide a specific incompatibility reason message for UNKNOWN-category dependency licenses during license checks.

Tests:

  • Add coverage for all project/dependency UNKNOWN category combinations and regression tests to ensure existing compatibility and incompatibility cases remain unchanged.

Split the UNKNOWN category check in getCompatibility() so that
dependencies with UNKNOWN license category are flagged as incompatible
when the project has a known category. Add a distinct reason message
for unrecognized licenses recommending manual review. Project-UNKNOWN
still returns 'unknown' to avoid false positives.

Implements TC-4706

Assisted-by: Claude Code
@sourcery-ai
Copy link
Copy Markdown

sourcery-ai Bot commented Jun 4, 2026

Reviewer's Guide

Adjusts license compatibility logic to treat UNKNOWN-category dependencies as incompatible when the project has a known license category, and introduces targeted tests and user-facing messaging for this new behavior while preserving UNKNOWN-project behavior.

Sequence diagram for runLicenseCheck incompatible UNKNOWN dependency reason

sequenceDiagram
  participant RunLicenseCheck
  participant GetCompatibility

  RunLicenseCheck->>GetCompatibility: getCompatibility(projectCategory, entry.category)
  GetCompatibility-->>RunLicenseCheck: status

  RunLicenseCheck->>RunLicenseCheck: [status === incompatible]
  alt entry.category.toUpperCase() === UNKNOWN
    RunLicenseCheck->>RunLicenseCheck: reason = License not recognized as a standard SPDX identifier. Manual review recommended to verify compatibility.
  else other incompatible
    RunLicenseCheck->>RunLicenseCheck: reason = Dependency license(s) are incompatible with the project license.
  end
  RunLicenseCheck->>RunLicenseCheck: incompatibleDependencies.push({ purl, licenses, category, reason })
Loading

File-Level Changes

Change Details Files
Refined license compatibility rules for UNKNOWN categories.
  • Changed getCompatibility so UNKNOWN project categories still return 'unknown' but UNKNOWN dependency categories with a known project category now return 'incompatible'.
  • Maintained existing restrictiveness comparison logic for known categories (PERMISSIVE, WEAK_COPYLEFT, STRONG_COPYLEFT) unchanged.
src/license/license_utils.js
Improved user-facing incompatibility reasons for UNKNOWN-category dependencies.
  • Updated runLicenseCheck to select a specific reason message when an incompatible dependency has category UNKNOWN, indicating the license is not a standard SPDX identifier and manual review is recommended.
  • Kept the previous generic incompatibility reason for all other incompatible dependencies.
src/license/index.js
Expanded test coverage for UNKNOWN-category license compatibility combinations.
  • Added a dedicated test suite for getCompatibility with UNKNOWN project/dependency categories, covering all relevant combinations and expected outcomes.
  • Included regression tests to ensure existing known-category compatibility and incompatibility behavior remains unchanged.
test/providers/license.test.js

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Copy link
Copy Markdown

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

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

Hey - I've found 1 issue

Prompt for AI Agents
Please address the comments from this code review:

## Individual Comments

### Comment 1
<location path="test/providers/license.test.js" line_range="67-73" />
<code_context>
 })

+suite('getCompatibility with UNKNOWN category', () => {
+	/** @type {Array<{proj: string, dep: string, expected: string}>} */
+	const cases = [
+		{ proj: 'PERMISSIVE', dep: 'UNKNOWN', expected: 'incompatible' },
+		{ proj: 'WEAK_COPYLEFT', dep: 'UNKNOWN', expected: 'incompatible' },
+		{ proj: 'STRONG_COPYLEFT', dep: 'UNKNOWN', expected: 'incompatible' },
+		{ proj: 'UNKNOWN', dep: 'PERMISSIVE', expected: 'unknown' },
+		{ proj: 'UNKNOWN', dep: 'UNKNOWN', expected: 'unknown' },
+	];
+
</code_context>
<issue_to_address>
**suggestion (testing):** Consider adding tests to confirm UNKNOWN-category handling is case-insensitive.

These cases exercise UNKNOWN vs known categories, but only with uppercase names. Since `getCompatibility` uses `toUpperCase()`, please add at least one test where `proj` and/or `dep` is `'unknown'` (lower or mixed case) to verify the UNKNOWN-specific branches behave correctly with varied input casing.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment thread test/providers/license.test.js
@a-oren
Copy link
Copy Markdown
Contributor Author

a-oren commented Jun 4, 2026

Verification Report for TC-4706 (commit 7ccd3fa)

Check Result Details
Review Feedback PASS 1 suggestion (case-insensitive tests); no code change requests
Root-Cause Investigation N/A No sub-tasks created
Scope Containment PASS PR files exactly match the 3 task-specified files
Diff Size PASS 3 files changed, proportionate to task scope
Commit Traceability PASS Commit 7ccd3fa references TC-4706
Sensitive Patterns PASS No secrets or credentials detected
CI Status PASS All 9 CI checks passed (lint/test Node 22+24, PR validation)
Acceptance Criteria PASS All 5 criteria satisfied
Test Quality WARN Test Documentation: uses /// (non-standard for JS, minor inconsistency); Repetitive Test Detection: PASS; Eval Quality: N/A
Test Change Classification ADDITIVE New test suite added, no existing tests modified
Verification Commands PASS npm test passed via CI

Overall: PASS

All checks pass. One minor note: test doc comments use /// style which is uncommon in this JS codebase (most tests use // or /** */), but this is informational only.


This comment was AI-generated by sdlc-workflow/verify-pr v0.9.1.

@a-oren a-oren requested a review from Strum355 June 4, 2026 18:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants