Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 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
Loading