diff --git a/CHANGELOG.md b/CHANGELOG.md index 791838e0..a525fe10 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,10 @@ All notable changes to SkillEvaluator are documented in this file. ### Fixed +- SPDX headers keep the full license expression, so `MIT OR GPL-3.0` is + no longer truncated to MIT and allowed. Closing comment markers such as + `*/` and `-->` are not treated as part of the expression + ([#86](https://github.com/NVIDIA/SkillEvaluator/issues/86)). - Windows personal-path PII now flags `C:\Users\...` usernames that start with `s` (for example `steve`), matching the intended whitespace class rather than excluding the letter `s` ([#87](https://github.com/NVIDIA/SkillEvaluator/issues/87)). diff --git a/src/skillevaluator/config/license_config.yaml b/src/skillevaluator/config/license_config.yaml index e7e16445..160123b6 100644 --- a/src/skillevaluator/config/license_config.yaml +++ b/src/skillevaluator/config/license_config.yaml @@ -212,7 +212,7 @@ proprietary_indicators: spdx_detection: # Regex pattern to extract SPDX identifier - pattern: "SPDX-License-Identifier:\\s*([A-Za-z0-9.+-]+)" + pattern: "SPDX-License-Identifier:\\s*([^\\n]+)" # Number of lines to scan from the beginning of each file scan_lines: 50 diff --git a/src/skillevaluator/constants.py b/src/skillevaluator/constants.py index 95eeb5c1..b9e78b18 100644 --- a/src/skillevaluator/constants.py +++ b/src/skillevaluator/constants.py @@ -219,7 +219,7 @@ LICENSE_HEADER_EXTENSIONS = {".py", ".sh", ".js", ".ts", ".yaml", ".yml", ".md", ".txt", ".json"} # SPDX header pattern for source files -SPDX_LICENSE_PATTERN = r"SPDX-License-Identifier:\s*([A-Za-z0-9.\-+]+)" +SPDX_LICENSE_PATTERN = r"SPDX-License-Identifier:\s*([^\n]+)" # ============================================================================= diff --git a/src/skillevaluator/validators/license.py b/src/skillevaluator/validators/license.py index 47fc188b..07eeb384 100644 --- a/src/skillevaluator/validators/license.py +++ b/src/skillevaluator/validators/license.py @@ -34,6 +34,18 @@ _SPDX_PATTERN = re.compile(SPDX_LICENSE_PATTERN, re.IGNORECASE) _LICENSE_SUFFIX_PATTERN = re.compile(r"(-license|-licence)$") _THE_PREFIX_PATTERN = re.compile(r"^(the-)?") +_SPDX_EXPRESSION_SPLIT = re.compile(r"\s+(?:OR|AND|WITH)\s+", re.IGNORECASE) + + +def _strip_spdx_capture(raw: str) -> str: + """Drop inline comment terminators and trailing source after the SPDX value.""" + raw = raw.split("#", 1)[0] + for marker in ("*/", "-->"): + idx = raw.find(marker) + if idx != -1: + raw = raw[:idx] + return raw.strip() + # File reference indicators in license field values _FILE_REFERENCE_KEYWORDS = frozenset(["see ", "refer to ", "license.txt", "license.md", "copying"]) @@ -510,11 +522,18 @@ def _extract_spdx_from_file(file_path: Path) -> str | None: with file_path.open(encoding="utf-8", errors="ignore") as f: header = "".join(islice(f, LICENSE_HEADER_SCAN_LINES)) if match := _SPDX_PATTERN.search(header): - return match.group(1).strip() + raw = _strip_spdx_capture(match.group(1)) + return raw or None except Exception as e: logger.debug("Could not scan %s: %s", file_path, e) return None + @staticmethod + def _expression_symbols(license_id: str) -> list[str]: + """Split an SPDX expression into identifiers, keeping hyphenated ids intact.""" + stripped = license_id.replace("(", " ").replace(")", " ").strip() + return [part.strip() for part in _SPDX_EXPRESSION_SPLIT.split(stripped) if part.strip()] + def _validate_license(self, detection: LicenseDetection, result: ValidationResult) -> None: """Validate detected license against allowlist/blocklist.""" license_id = detection.license_id @@ -522,6 +541,11 @@ def _validate_license(self, detection: LicenseDetection, result: ValidationResul result.add_warning("License detection returned empty identifier") return + symbols = self._expression_symbols(license_id) + if len(symbols) > 1: + self._validate_license_expression(detection, symbols, result) + return + normalized = self._normalize_license_id(license_id) if normalized in self._normalized_allowlist: @@ -534,6 +558,32 @@ def _validate_license(self, detection: LicenseDetection, result: ValidationResul self._set_license_metadata(result, detection, "unknown") self._handle_unknown_license(result, license_id, detection.file_path) + def _validate_license_expression( + self, + detection: LicenseDetection, + symbols: list[str], + result: ValidationResult, + ) -> None: + """Fail closed if any SPDX expression symbol is blocked or unknown when required.""" + blocked = [symbol for symbol in symbols if self._normalize_license_id(symbol) in self._normalized_blocklist] + if blocked: + self._set_license_metadata(result, detection, "blocked") + result.add_message( + f"SPDX expression '{detection.license_id}' includes blocked license '{blocked[0]}'" + ) + self._add_blocked_license_finding(result, blocked[0], detection.file_path) + return + if all(self._normalize_license_id(symbol) in self._normalized_allowlist for symbol in symbols): + self._set_license_metadata(result, detection, "allowed") + result.add_message(f"License: {detection.license_id} (ALLOWED - permissive)") + return + unknown = next( + (symbol for symbol in symbols if self._normalize_license_id(symbol) not in self._normalized_allowlist), + detection.license_id, + ) + self._set_license_metadata(result, detection, "unknown") + self._handle_unknown_license(result, unknown, detection.file_path) + @staticmethod def _set_license_metadata(result: ValidationResult, detection: LicenseDetection, status: str) -> None: """Set license metadata on validation result.""" diff --git a/tests/validators/test_license.py b/tests/validators/test_license.py index 5bec0c2f..254d5a73 100644 --- a/tests/validators/test_license.py +++ b/tests/validators/test_license.py @@ -632,6 +632,130 @@ def test_multiple_files_same_license(self, tmp_path: Path): assert result.passed assert "MIT" in result.metadata.get("license", "") + def test_spdx_or_expression_with_gpl_is_blocked(self, tmp_path: Path): + """MIT OR GPL-3.0 in an SPDX header must not be truncated to allowed MIT (#86).""" + skill_dir = tmp_path / "spdx-or-gpl" + skill_dir.mkdir() + (skill_dir / "SKILL.md").write_text("""--- +name: spdx-or-gpl +description: Skill whose only license signal is an SPDX OR expression +--- + +# SPDX OR +""") + (skill_dir / "script.py").write_text("""# SPDX-License-Identifier: MIT OR GPL-3.0 +print("hi") +""") + + result = LicenseValidator().validate(skill_dir) + + assert not result.passed + assert result.metadata.get("license_status") == "blocked" + assert "GPL-3.0" in (result.metadata.get("license") or "") + assert any(f.check_name == "blocked_license" for f in result.findings) + assert not any("License: MIT (ALLOWED" in message for message in result.messages) + + def test_spdx_or_expression_in_block_comment_is_blocked(self, tmp_path: Path): + """`*/` after MIT OR GPL-3.0 must not turn the GPL half into an unknown id.""" + skill_dir = tmp_path / "spdx-js-block" + skill_dir.mkdir() + (skill_dir / "SKILL.md").write_text("""--- +name: spdx-js-block +description: Skill whose SPDX header is inside a JavaScript block comment +--- + +# SPDX JS +""") + (skill_dir / "script.js").write_text("/* SPDX-License-Identifier: MIT OR GPL-3.0 */\n") + + result = LicenseValidator().validate(skill_dir) + + assert not result.passed + assert result.metadata.get("license_status") == "blocked" + assert "GPL-3.0" in (result.metadata.get("license") or "") + assert "*/" not in (result.metadata.get("license") or "") + assert any(f.check_name == "blocked_license" for f in result.findings) + + def test_spdx_inline_block_comment_with_code_suffix_is_blocked(self, tmp_path: Path): + """Inline `*/` plus trailing code must not leave GPL-3.0 */ attached to the symbol.""" + skill_dir = tmp_path / "spdx-js-inline" + skill_dir.mkdir() + (skill_dir / "SKILL.md").write_text("""--- +name: spdx-js-inline +description: Skill whose SPDX header shares a line with trailing code +--- + +# SPDX inline +""") + (skill_dir / "script.js").write_text("/* SPDX-License-Identifier: MIT OR GPL-3.0 */ const x = 1;\n") + + result = LicenseValidator().validate(skill_dir) + + assert not result.passed + assert result.metadata.get("license_status") == "blocked" + assert "GPL-3.0" in (result.metadata.get("license") or "") + assert "*/" not in (result.metadata.get("license") or "") + assert any(f.check_name == "blocked_license" for f in result.findings) + + def test_spdx_or_expression_in_html_comment_is_blocked(self, tmp_path: Path): + """`-->` after MIT OR GPL-3.0 must not hide the GPL half.""" + skill_dir = tmp_path / "spdx-html-comment" + skill_dir.mkdir() + (skill_dir / "SKILL.md").write_text("""--- +name: spdx-html-comment +description: Skill whose SPDX header is inside an HTML comment +--- + +# SPDX HTML +""") + (skill_dir / "notes.md").write_text("\n") + + result = LicenseValidator().validate(skill_dir) + + assert not result.passed + assert result.metadata.get("license_status") == "blocked" + assert "GPL-3.0" in (result.metadata.get("license") or "") + assert "-->" not in (result.metadata.get("license") or "") + assert any(f.check_name == "blocked_license" for f in result.findings) + + def test_spdx_and_expression_with_gpl_is_blocked(self, tmp_path: Path): + """Apache-2.0 AND GPL-3.0 must evaluate the copyleft half, not only Apache-2.0.""" + skill_dir = tmp_path / "spdx-and-gpl" + skill_dir.mkdir() + (skill_dir / "SKILL.md").write_text("""--- +name: spdx-and-gpl +description: Skill whose only license signal is an SPDX AND expression +--- + +# SPDX AND +""") + (skill_dir / "script.py").write_text("# SPDX-License-Identifier: Apache-2.0 AND GPL-3.0\n") + + result = LicenseValidator().validate(skill_dir) + + assert not result.passed + assert any(f.check_name == "blocked_license" for f in result.findings) + assert result.metadata.get("license_status") != "allowed" + + def test_spdx_or_expression_of_allowed_licenses_passes(self, tmp_path: Path): + """Compound expressions stay allowed when every symbol is permissive.""" + skill_dir = tmp_path / "spdx-or-mit" + skill_dir.mkdir() + (skill_dir / "SKILL.md").write_text("""--- +name: spdx-or-mit +description: Skill whose SPDX header is MIT OR MIT-0 +--- + +# SPDX OR allowed +""") + (skill_dir / "script.py").write_text("# SPDX-License-Identifier: MIT OR MIT-0\n") + + result = LicenseValidator().validate(skill_dir) + + assert result.passed + assert result.metadata.get("license_status") == "allowed" + assert "MIT OR MIT-0" in (result.metadata.get("license") or "") + # ============================================================================= # NO LICENSE / UNKNOWN LICENSE TESTS