Skip to content

fix: normalize multi-skill risk_score before threshold check - #368

Open
andrewwhitecdw wants to merge 2 commits into
NVIDIA:mainfrom
andrewwhitecdw:codequality/cli-normalize-multi-skill-risk-score
Open

fix: normalize multi-skill risk_score before threshold check#368
andrewwhitecdw wants to merge 2 commits into
NVIDIA:mainfrom
andrewwhitecdw:codequality/cli-normalize-multi-skill-risk-score

Conversation

@andrewwhitecdw

Copy link
Copy Markdown

This PR addresses the following issue in src/skillspector/cli.py: normalize multi-skill risk_score before threshold check.

Changes

  • src/skillspector/cli.py: normalize multi-skill risk_score before threshold check.

Details

--- a/src/skillspector/cli.py
+++ b/src/skillspector/cli.py
@@ -1,3 +1,7 @@
-            score = result.get("risk_score") or 0
-            if isinstance(score, int) and score > max_score:
-                max_score = score
+            score = result.get("risk_score") or 0
+            try:
+                score = int(score)
+            except (TypeError, ValueError):
+                score = 0
+            if score > max_score:
+                max_score = score

Tests

  • tests/test_cli.py
--- a/tests/test_cli.py
+++ b/tests/test_cli.py
@@ -1,6 +1,7 @@
 # Tests for skillspector.cli
 
 import pytest
+import typer
 
 
 def test_example_placeholder():
@@ -10,3 +11,32 @@
 def test_another_example():
     # Existing placeholder test
     assert True
+
+
+def test_multi_skill_string_risk_score_counts_toward_exit(monkeypatch, tmp_path):
+    """A risk_score stored as a string must still affect the aggregate exit code."""
+    from skillspector.cli import _scan_multi_skill, FormatChoice
+    from skillspector.multi_skill import MultiSkillDetectionResult, SkillInfo
+
+    skill = SkillInfo(name="bad-skill", path=tmp_path, relative_path="bad-skill")
+    detection = MultiSkillDetectionResult(
+        skills=[skill],
+        is_multi_skill=True,
+        has_root_skill=False,
+    )
+
+    def fake_invoke(state, config=None):
+        return {
+            "risk_score": "85",
+            "risk_severity": "HIGH",
+            "findings": [{"id": "f1"}],
+        }
+
+    monkeypatch.setattr(
+        "skillspector.cli.graph",
+        type("Graph", (), {"invoke": staticmethod(fake_invoke)})(),
+    )
+
+    with pytest.raises(typer.Exit) as exc_info:
+        _scan_multi_skill(detection, FormatChoice.terminal, None, True, None, False)
+
+    assert exc_info.value.exit_code == 1

Contributor guidelines

Per this repo's CONTRIBUTING.md:

  • All commits are signed off (Signed-off-by trailer, DCO).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants