Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@ All notable changes to SkillEvaluator are documented in this file.

### Fixed

- Malformed, non-UTF-8, or unreadable bundled and custom policy files now
produce path-specific CLI errors instead of leaking raw parser or I/O errors
([#128](https://github.com/NVIDIA/SkillEvaluator/issues/128)).
- License detection no longer treats a frontmatter `license` identifier as
authoritative when a LICENSE file declares a different license. Claiming
MIT while shipping GPL-3.0 now fails closed. Every LICENSE/COPYING file is
Expand Down
19 changes: 15 additions & 4 deletions src/skillevaluator/validators/policy.py
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,19 @@ def _coerce_email_regex(value: Any, source: str) -> re.Pattern[str] | None:
_KNOWN_IDENTITY_KEYS = {"author_email_regex"}


def _load_policy_yaml(path: Path) -> Any:
"""Load policy YAML and normalize parser failures to the public contract."""
try:
with path.open(encoding="utf-8") as fh:
return yaml.safe_load(fh) or {}
except (yaml.YAMLError, UnicodeError, RecursionError) as exc:
raise ValueError(f"Invalid policy YAML in {path}: {exc}") from exc
except FileNotFoundError:
raise
except OSError as exc:
raise ValueError(f"Could not read policy file {path}: {exc}") from exc


def _warn_unknown_keys(data: dict[str, Any], known: set[str], context: str, source: str) -> None:
unknown = set(data) - known
if unknown:
Expand Down Expand Up @@ -227,8 +240,7 @@ def load_profile(name: str = DEFAULT_PROFILE_NAME) -> ValidationPolicy:
if not path.exists():
available = sorted(p.stem for p in PROFILES_DIR.glob("*.yaml")) if PROFILES_DIR.exists() else []
raise FileNotFoundError(f"Unknown profile {name!r}. Available bundled profiles: {available or '(none)'}")
with path.open(encoding="utf-8") as fh:
data = yaml.safe_load(fh) or {}
data = _load_policy_yaml(path)
return _policy_from_data(data, fallback_profile=name, source=path)


Expand All @@ -248,8 +260,7 @@ def load_policy_file(
raise FileNotFoundError(f"Custom policy file not found: {path}")

base = load_profile(base_profile)
with path.open(encoding="utf-8") as fh:
custom_data = yaml.safe_load(fh) or {}
custom_data = _load_policy_yaml(path)
custom = _policy_from_data(custom_data, fallback_profile=base.profile, source=path)

merged_overrides = dict(base.severity_overrides)
Expand Down
23 changes: 23 additions & 0 deletions tests/test_commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,29 @@ def test_validate_fixture_no_llm() -> None:
assert "All validations passed" in result.output


def test_validate_reports_malformed_policy_without_traceback(tmp_path: Path) -> None:
policy = tmp_path / "broken-policy.yaml"
policy.write_text("severity_overrides: [", encoding="utf-8")

result = CliRunner().invoke(
cli,
[
"validate",
str(FIXTURE),
"--no-llm",
"--no-dedup",
"--checks",
"schema",
"--policy",
str(policy),
],
)

assert result.exit_code == 1
assert f"Invalid policy YAML in {policy}" in result.output
assert isinstance(result.exception, SystemExit)


def test_validate_prints_tier1_section_banner() -> None:
# The Tier 1 section is announced as it runs so it is visibly reported in
# CI logs (SkillEvaluator parity), not only inside the final combined report.
Expand Down
71 changes: 71 additions & 0 deletions tests/validators/test_policy.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,10 @@
from pathlib import Path

import pytest
import yaml

from skillevaluator.models.result import Severity
from skillevaluator.validators import policy as policy_module
from skillevaluator.validators.policy import (
DEFAULT_PROFILE_NAME,
ValidationPolicy,
Expand Down Expand Up @@ -46,6 +48,75 @@ def test_custom_policy_overlays_the_public_profile(tmp_path: Path) -> None:
assert policy.severity_for("SCHEMA", "author_missing", Severity.LOW) == Severity.HIGH


def test_custom_policy_wraps_malformed_yaml(tmp_path: Path) -> None:
custom = tmp_path / "broken-policy.yaml"
custom.write_text("severity_overrides: [", encoding="utf-8")

with pytest.raises(ValueError) as exc_info:
load_policy_file(custom)

assert f"Invalid policy YAML in {custom}" in str(exc_info.value)
assert isinstance(exc_info.value.__cause__, yaml.YAMLError)


def test_custom_policy_wraps_invalid_encoding(tmp_path: Path) -> None:
custom = tmp_path / "utf16-policy.yaml"
custom.write_bytes("profile: encoded\n".encode("utf-16"))

with pytest.raises(ValueError) as exc_info:
load_policy_file(custom)

assert f"Invalid policy YAML in {custom}" in str(exc_info.value)
assert isinstance(exc_info.value.__cause__, UnicodeDecodeError)


def test_custom_policy_wraps_read_errors(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None:
custom = tmp_path / "unreadable-policy.yaml"
custom.write_text("profile: unreadable\n", encoding="utf-8")
original_open = Path.open

def deny_custom_policy(path: Path, *args: object, **kwargs: object):
if path == custom:
raise PermissionError("access denied")
return original_open(path, *args, **kwargs)

monkeypatch.setattr(Path, "open", deny_custom_policy)

with pytest.raises(ValueError) as exc_info:
load_policy_file(custom)

assert f"Could not read policy file {custom}" in str(exc_info.value)
assert isinstance(exc_info.value.__cause__, PermissionError)


def test_policy_yaml_wraps_recursion_errors(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None:
policy = tmp_path / "deeply-nested-policy.yaml"
policy.write_text("severity_overrides: []\n", encoding="utf-8")

def exhaust_parser(_stream: object) -> object:
raise RecursionError("maximum recursion depth exceeded")

monkeypatch.setattr(policy_module.yaml, "safe_load", exhaust_parser)

with pytest.raises(ValueError) as exc_info:
policy_module._load_policy_yaml(policy)

assert f"Invalid policy YAML in {policy}" in str(exc_info.value)
assert isinstance(exc_info.value.__cause__, RecursionError)


def test_bundled_profile_wraps_malformed_yaml(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None:
profile = tmp_path / "broken.yaml"
profile.write_text("identity: [", encoding="utf-8")
monkeypatch.setattr(policy_module, "PROFILES_DIR", tmp_path)

with pytest.raises(ValueError) as exc_info:
load_profile("broken")

assert f"Invalid policy YAML in {profile}" in str(exc_info.value)
assert isinstance(exc_info.value.__cause__, yaml.YAMLError)


def test_policy_validation_and_resolution() -> None:
assert resolve_policy().profile == "external"
assert (
Expand Down