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 @@ -24,6 +24,9 @@ All notable changes to SkillEvaluator are documented in this file.

### Fixed

- Fully covered documentation-only skills no longer fail security validation
solely because non-applicable SkillSpector analyzers report a partial status
([#137](https://github.com/NVIDIA/SkillEvaluator/issues/137)).
- 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)).
Expand Down
26 changes: 24 additions & 2 deletions src/skillevaluator/validators/security.py
Original file line number Diff line number Diff line change
Expand Up @@ -1214,7 +1214,29 @@ def _validate_skillspector_report(
"missing required analyzer evidence; security scan did not complete"
)
return False
if (is_complete or uses_statusless_completeness_schema) and counts["total_components"]:
analyzer_states = {
state
for evidence in analyzer_evidence.values()
for state, _accounting in evidence
}
complete_by_applicability = (
uses_completeness_schema
and not is_complete
and completeness_status == "partial"
and coverage_percent == 100
and counts["partially_inspected_files"] == 0
and counts["entirely_uninspected_files"] == 0
and not ledger_exceptions
and not limitations
and report_metadata.get("has_executable_scripts") is False
and "not_applicable" in analyzer_states
Comment thread
deepujain marked this conversation as resolved.
Outdated
and analyzer_states <= {"completed", "not_applicable"}
)
if (
is_complete
or uses_statusless_completeness_schema
or complete_by_applicability
) and counts["total_components"]:
universal_analyzer_ids = _SKILLSPECTOR_COMMON_UNIVERSAL_ANALYZERS | (
{"artifact_integrity"} if uses_completeness_schema else set()
)
Expand Down Expand Up @@ -1307,7 +1329,7 @@ def _validate_skillspector_report(
"security scan did not complete"
)
return False
else:
elif not complete_by_applicability:
if not (
counts["partially_inspected_files"]
or counts["entirely_uninspected_files"]
Expand Down
97 changes: 97 additions & 0 deletions tests/validators/test_security.py
Original file line number Diff line number Diff line change
Expand Up @@ -2206,6 +2206,103 @@ def test_skillspector_accepts_valid_clean_report(
assert not result.errors
assert any(detail.check_name == "skillspector" for detail in result.success_details)

@patch("skillevaluator.validators.security.Tools")
def test_skillspector_accepts_fully_covered_documentation_only_report(
self,
mock_tools,
sample_skill_dir: Path,
) -> None:
payload = _skillspector_json_report()
payload["metadata"]["skillspector_version"] = "2.11.2"
payload["components"] = [{"path": "SKILL.md", "executable": False}]
payload["analysis_completeness"].update(
{
"total_components": 1,
"scanned_components": 1,
"fully_inspected_files": 1,
"is_complete": False,
"status": "partial",
}
)
payload["analysis_completeness"]["analyzer_statuses"].append(
{
"analyzer_id": "bundled_execution_surface",
"status": "not_applicable",
"planned_work": 0,
"completed": 0,
"partial": 0,
"skipped": 0,
"failed": 0,
"unaccounted": 0,
}
)
for status in payload["analysis_completeness"]["analyzer_statuses"]:
if status["analyzer_id"] in {
"behavioral_ast",
"behavioral_taint_tracking",
"mcp_least_privilege",
"meta_analyzer",
}:
status.update({"status": "not_applicable", "planned_work": 0, "completed": 0})
_set_universal_analyzer_work(payload)

result = _validate_skillspector_payload(mock_tools, sample_skill_dir, payload)

assert result.passed
assert not result.errors
assert any(detail.check_name == "skillspector" for detail in result.success_details)

@pytest.mark.parametrize(
"incomplete_detail",
[
pytest.param({"coverage_percent": 0}, id="missing-coverage"),
pytest.param({"partially_inspected_files": 1}, id="partially-inspected"),
pytest.param({"limitations": ["Analyzer failed."]}, id="limitation"),
pytest.param({"ledger_exceptions": [{"fatal": False}]}, id="ledger-exception"),
],
)
@patch("skillevaluator.validators.security.Tools")
def test_skillspector_documentation_only_exception_requires_full_coverage(
self,
mock_tools,
sample_skill_dir: Path,
incomplete_detail: dict,
) -> None:
payload = _skillspector_json_report()
payload["analysis_completeness"].update(
{
"is_complete": False,
"status": "partial",
**incomplete_detail,
}
)
payload["analysis_completeness"]["analyzer_statuses"][0].update(
{"status": "not_applicable", "planned_work": 0, "completed": 0}
)

result = _validate_skillspector_payload(mock_tools, sample_skill_dir, payload)

assert result.is_incomplete
assert not any(detail.check_name == "skillspector" for detail in result.success_details)

@patch("skillevaluator.validators.security.Tools")
def test_skillspector_executable_skill_cannot_use_documentation_only_exception(
self,
mock_tools,
sample_skill_dir: Path,
) -> None:
payload = _skillspector_json_report()
payload["metadata"]["has_executable_scripts"] = True
payload["analysis_completeness"].update({"is_complete": False, "status": "partial"})
payload["analysis_completeness"]["analyzer_statuses"][0].update(
{"status": "not_applicable", "planned_work": 0, "completed": 0}
)

result = _validate_skillspector_payload(mock_tools, sample_skill_dir, payload)

assert result.is_incomplete
assert not any(detail.check_name == "skillspector" for detail in result.success_details)

@pytest.mark.parametrize("version", ["2.9.5-safe", "2.9.6", "2.11.1-safe"])
@patch("skillevaluator.validators.security.Tools")
def test_skillspector_accepts_captured_no_llm_report(
Expand Down
Loading