From d2f1319c43fabb77be398687ca68f44cc3d2ae9e Mon Sep 17 00:00:00 2001 From: mimran-khan Date: Thu, 27 Aug 2026 01:26:13 +0530 Subject: [PATCH 1/2] fix: evaluate full SPDX license expressions The header regex stopped at the first identifier, so MIT OR GPL-3.0 was recorded as MIT and allowed. Capture the whole expression and fail closed if any symbol is blocked. Fixes #86 Signed-off-by: mimran-khan --- CHANGELOG.md | 6 +- src/skillevaluator/config/license_config.yaml | 2 +- src/skillevaluator/constants.py | 2 +- src/skillevaluator/validators/license.py | 40 +++++++++++- tests/validators/test_license.py | 61 +++++++++++++++++++ 5 files changed, 105 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a2ad6515..be1a93a6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,9 +4,9 @@ All notable changes to SkillEvaluator are documented in this file. ## Unreleased -### Fixed - -- Tier 3 paired pass@k evidence now respects Python's active integer-string +- SPDX headers keep the full license expression, so `MIT OR GPL-3.0` is + no longer truncated to MIT and allowed + ([#86](https://github.com/NVIDIA/SkillEvaluator/issues/86)). conversion limit, preserves nonzero Wilson interval widths and paired-effect directions at large case counts, and documents exact-rational omission markers. 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 5926ba41..8a303151 100644 --- a/src/skillevaluator/constants.py +++ b/src/skillevaluator/constants.py @@ -215,7 +215,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 5541723a..f9b47a77 100644 --- a/src/skillevaluator/validators/license.py +++ b/src/skillevaluator/validators/license.py @@ -34,6 +34,7 @@ _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) # File reference indicators in license field values _FILE_REFERENCE_KEYWORDS = frozenset(["see ", "refer to ", "license.txt", "license.md", "copying"]) @@ -309,11 +310,17 @@ 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() + return match.group(1).split("#", 1)[0].strip() 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 @@ -321,6 +328,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: @@ -333,6 +345,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 da33af8d..58a7a471 100644 --- a/tests/validators/test_license.py +++ b/tests/validators/test_license.py @@ -465,6 +465,67 @@ 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_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 From 0f263c213143c59dad6fffe06d03ff8f57d7dc96 Mon Sep 17 00:00:00 2001 From: mimran-khan Date: Fri, 28 Aug 2026 13:58:28 +0530 Subject: [PATCH 2/2] fix: strip comment closers from SPDX license expressions A JS or HTML SPDX header captured `MIT OR GPL-3.0 */` (or `-->`), so the GPL half looked unknown and default validation warned instead of blocking. Drop `*/` and `-->` before splitting the expression. Fixes #86 Signed-off-by: mimran-khan --- src/skillevaluator/validators/license.py | 5 ++- tests/validators/test_license.py | 42 ++++++++++++++++++++++++ 2 files changed, 46 insertions(+), 1 deletion(-) diff --git a/src/skillevaluator/validators/license.py b/src/skillevaluator/validators/license.py index f9b47a77..834892c4 100644 --- a/src/skillevaluator/validators/license.py +++ b/src/skillevaluator/validators/license.py @@ -35,6 +35,7 @@ _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) +_SPDX_COMMENT_CLOSE = re.compile(r"\s*(?:\*/|-->)\s*$") # File reference indicators in license field values _FILE_REFERENCE_KEYWORDS = frozenset(["see ", "refer to ", "license.txt", "license.md", "copying"]) @@ -310,7 +311,9 @@ 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).split("#", 1)[0].strip() or None + raw = match.group(1).split("#", 1)[0] + raw = _SPDX_COMMENT_CLOSE.sub("", raw).strip() + return raw or None except Exception as e: logger.debug("Could not scan %s: %s", file_path, e) return None diff --git a/tests/validators/test_license.py b/tests/validators/test_license.py index 58a7a471..47df7884 100644 --- a/tests/validators/test_license.py +++ b/tests/validators/test_license.py @@ -488,6 +488,48 @@ def test_spdx_or_expression_with_gpl_is_blocked(self, tmp_path: Path): 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_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"