diff --git a/hermes_cli/config_defaults.py b/hermes_cli/config_defaults.py index 1c10c55989..c2e43b4536 100644 --- a/hermes_cli/config_defaults.py +++ b/hermes_cli/config_defaults.py @@ -2004,7 +2004,7 @@ DEFAULT_CONFIG = { # guard (which stays the enforcement layer) and only when the # optional `skillevaluator` binary is on PATH: # uv tool install --python 3.13 \ - # "skillevaluator @ git+https://github.com/NVIDIA/SkillEvaluator.git" + # "skillevaluator @ git+https://github.com/NVIDIA/SkillEvaluator.git@v0.1.0" # Findings are informational — shown with file/line before the # install confirmation, never blocking. Secrets-class findings # (private keys, tokens, credentialed connection strings) are diff --git a/tests/tools/test_skillevaluator_scan.py b/tests/tools/test_skillevaluator_scan.py index 9a3f055898..4ae22ca9e4 100644 --- a/tests/tools/test_skillevaluator_scan.py +++ b/tests/tools/test_skillevaluator_scan.py @@ -102,7 +102,9 @@ class TestParseReport: assert report.findings == [] assert report.incomplete_checks == ["Security Scan"] - def test_incomplete_check_findings_not_collected(self): + def test_incomplete_check_findings_preserved(self): + """Partial evidence from an incomplete validator is kept as findings + (Nir Paz review) — only the validator's pass/fail verdict is excluded.""" raw = _report_json([]) raw["results"].append({ "validator": "Security Scan", @@ -111,7 +113,23 @@ class TestParseReport: "findings": [_finding("hardcoded_secrets", severity="critical")], }) report = _parse_report(raw) - assert report.findings == [] + assert len(report.findings) == 1 + assert report.findings[0].is_secrets_class + assert report.incomplete_checks == ["Security Scan"] + # findings present -> report is not clean, even though the only + # failing validator was incomplete + assert not report.passed + + def test_incomplete_check_without_findings_stays_passed(self): + raw = _report_json([]) + raw["results"].append({ + "validator": "Security Scan", + "passed": False, + "status": "incomplete", + "findings": [], + }) + report = _parse_report(raw) + assert report.passed def test_complete_failed_check_still_fails(self): report = _parse_report(_report_json([_finding("emails")])) diff --git a/tools/skillevaluator_scan.py b/tools/skillevaluator_scan.py index 32c118daec..bcf00b4086 100644 --- a/tools/skillevaluator_scan.py +++ b/tools/skillevaluator_scan.py @@ -28,7 +28,7 @@ Design contract (deliberate): The scanner binary is optional:: uv tool install --python 3.13 \ - "skillevaluator @ git+https://github.com/NVIDIA/SkillEvaluator.git" + "skillevaluator @ git+https://github.com/NVIDIA/SkillEvaluator.git@v0.1.0" Enable/disable via ``skills.tier1_advisory`` in config.yaml (default: on; a no-op unless the binary is installed). @@ -57,7 +57,7 @@ SCANNER_NAME = "skillevaluator-tier1" # # `security` invokes NVIDIA SkillSpector (a second optional binary, # pinned separately: uv tool install -# "git+https://github.com/NVIDIA/SkillSpector.git") in its static-rules +# "git+https://github.com/NVIDIA/SkillSpector.git@v2.9.5") in its static-rules # mode — still keyless, no LLM calls. When SkillSpector is absent or its # report fails SkillEvaluator's internal consistency checks, the check # reports status="incomplete" and is treated as "no opinion" here. @@ -143,22 +143,22 @@ def tier1_advisory_enabled() -> bool: def _parse_report(report: dict) -> Tier1Report: """Reduce a SkillEvaluator JSON report to install-relevant findings. - A validator whose ``status`` is ``"incomplete"`` produced no usable - evidence (e.g. SkillSpector missing, or its report failed - SkillEvaluator's internal consistency checks) — its fail verdict - carries no findings, so it is recorded as an incomplete check and - excluded from the pass/fail signal rather than rendered as an - unexplained failure. + A validator whose ``status`` is ``"incomplete"`` produced partial + evidence at best (e.g. SkillSpector missing, or its report failed + SkillEvaluator's internal consistency checks). Its findings ARE + kept — partial evidence is still evidence — but the validator is + excluded from the pass/fail signal, so an evidence-free fail + verdict can't render as an unexplained failure. """ findings: List[Tier1Finding] = [] incomplete: List[str] = [] any_complete_failed = False for res in report.get("results", []) or []: validator = str(res.get("validator", "unknown")) - if str(res.get("status", "")).lower() == "incomplete": + is_incomplete = str(res.get("status", "")).lower() == "incomplete" + if is_incomplete: incomplete.append(validator) - continue - if not res.get("passed", True): + elif not res.get("passed", True): any_complete_failed = True for f in res.get("findings", []) or []: if not isinstance(f, dict): @@ -219,7 +219,10 @@ def format_tier1_report(report: Tier1Report, limit: int = 10) -> str: return "" lines: List[str] = [] if not report.findings: - lines.append("SkillEvaluator Tier 1: no findings.") + if report.incomplete_checks: + lines.append("SkillEvaluator Tier 1: no findings from completed checks.") + else: + lines.append("SkillEvaluator Tier 1: no findings.") else: lines.append( f"SkillEvaluator Tier 1 (advisory): " diff --git a/website/docs/user-guide/features/skills.md b/website/docs/user-guide/features/skills.md index 6a2e987784..ec71205d0c 100644 --- a/website/docs/user-guide/features/skills.md +++ b/website/docs/user-guide/features/skills.md @@ -355,8 +355,8 @@ the `security` check; without it that check simply reports "not run"): ```bash uv tool install --python 3.13 \ - "skillevaluator @ git+https://github.com/NVIDIA/SkillEvaluator.git" -uv tool install "git+https://github.com/NVIDIA/SkillSpector.git" + "skillevaluator @ git+https://github.com/NVIDIA/SkillEvaluator.git@v0.1.0" +uv tool install "git+https://github.com/NVIDIA/SkillSpector.git@v2.9.5" ``` Without the binary on PATH the scan is silently skipped. To turn it off