refactor: Clean up credential validation output

r0BIT · r0BIT · commit 267615531582 · 2025-12-11T23:48:46.000+01:00
- Remove legacy [+], [-], [?] prefixes (Rich handles coloring now)
- Update color detection to use VALID/INVALID/BLOCKED/UNKNOWN keywords
- Messages are now cleaner: 'VALID', 'INVALID (wrong password)', etc.
diff --git a/taskhound/classification.py b/taskhound/classification.py
@@ -93,7 +93,7 @@ def _analyze_password_age(
     if hv and hv.loaded:
         risk_level, pwd_analysis = hv.analyze_password_age(runas, task_date)
         if risk_level != "UNKNOWN":
-            return pwd_analysis
+            return f"{risk_level}: {pwd_analysis}"
 
     # Fall back to pre-fetched LDAP data if BloodHound not available
     if pwd_cache and task_date:
@@ -108,7 +108,7 @@ def _analyze_password_age(
             if pwd_last_set:
                 risk_level, pwd_analysis = _analyze_password_freshness(task_date, pwd_last_set)
                 if risk_level != "UNKNOWN":
-                    return pwd_analysis
+                    return f"{risk_level}: {pwd_analysis}"
         except Exception as e:
             from .utils.logging import debug
             debug(f"Password analysis failed for {runas}: {e}")
diff --git a/taskhound/output/printer.py b/taskhound/output/printer.py
@@ -64,17 +64,22 @@ def print_task_table(
         if label == "Decrypted Pwd" and value:
             value_style = COLORS["password"]
         elif label == "Cred Validation":
-            if "[+]" in value:
+            if "VALID" in value.upper() and "INVALID" not in value.upper():
                 value_style = COLORS["success"]
-            elif "[-]" in value:
+            elif "INVALID" in value.upper() or "BLOCKED" in value.upper():
                 value_style = COLORS["error"]
-            elif "[?]" in value:
+            elif "UNKNOWN" in value.upper():
                 value_style = COLORS["warning"]
         elif label == "Pwd Analysis":
             if "GOOD" in value.upper() or "newer" in value.lower():
                 value_style = COLORS["success"]
             elif "BAD" in value.upper() or "stale" in value.lower():
                 value_style = COLORS["warning"]
+            # Strip the GOOD:/BAD: prefix from display (used only for color detection)
+            if value.upper().startswith("GOOD: "):
+                value = value[6:]
+            elif value.upper().startswith("BAD: "):
+                value = value[5:]
         elif label == "Enabled":
             if value.lower() == "true":
                 value_style = COLORS["success"]
@@ -353,19 +358,19 @@ def format_block(
         # Build status display
         if cred_status == "unknown":
             if password_analysis and "GOOD" in password_analysis.upper():
-                status_display = "[+] LIKELY VALID (task never ran, but password newer than pwdLastSet)"
+                status_display = "LIKELY VALID (task never ran, password newer than pwdLastSet)"
             elif password_analysis and "BAD" in password_analysis.upper():
-                status_display = "[-] LIKELY INVALID (task never ran, password older than pwdLastSet)"
+                status_display = "LIKELY INVALID (task never ran, password older than pwdLastSet)"
             else:
-                status_display = f"[?] UNKNOWN - task never ran ({cred_code})"
+                status_display = f"UNKNOWN - task never ran ({cred_code})"
         elif cred_valid is True:
-            status_display = "[+] VALID (hijackable)" if cred_hijackable else f"[+] VALID (restricted: {cred_status})"
+            status_display = "VALID" if cred_hijackable else f"VALID (restricted: {cred_status})"
         elif cred_status == "invalid":
-            status_display = "[-] INVALID (wrong password)"
+            status_display = "INVALID (wrong password)"
         elif cred_status == "blocked":
-            status_display = "[-] BLOCKED (account disabled/expired)"
+            status_display = "BLOCKED (account disabled/expired)"
         else:
-            status_display = f"[?] {cred_status} ({cred_code})"
+            status_display = f"{cred_status} ({cred_code})"
 
         rows.append(("Cred Validation", status_display))
 
diff --git a/tests/test_classification.py b/tests/test_classification.py
@@ -76,7 +76,7 @@ def test_returns_analysis_when_risk_known(self):
         meta = {"date": "2024-01-01T00:00:00"}
         result = _analyze_password_age(hv, "user@domain.local", meta, "\\Task")
 
-        assert result == "Password 180+ days old"
+        assert result == "HIGH: Password 180+ days old"
         hv.analyze_password_age.assert_called_once()
 
     def test_returns_none_when_risk_unknown(self):
@@ -104,7 +104,7 @@ def test_warns_when_using_fallback_date(self):
             assert "no explicit creation date" in mock_warn.call_args[0][0]
             assert "TestTask" in mock_warn.call_args[0][0]
             # Should still return the analysis result even with fallback warning
-            assert result == "Analysis result"
+            assert result == "HIGH: Analysis result"
 
 
 class TestClassifyTask:
@@ -137,7 +137,7 @@ def test_tier0_with_stored_credentials(self):
 
         assert result.task_type == "TIER-0"
         assert result.reason == "Domain Admin member"
-        assert result.password_analysis == "Password old"
+        assert result.password_analysis == "HIGH: Password old"
         assert result.should_include is True
         assert row.type == TaskType.TIER0.value
 
@@ -426,7 +426,7 @@ def test_pwd_cache_fallback_when_no_bloodhound(self):
                 pwd_cache=pwd_cache,
             )
             
-            assert result == "Password changed after task creation"
+            assert result == "GOOD: Password changed after task creation"
             mock_analyze.assert_called_once()
 
     def test_pwd_cache_normalizes_domain_username(self):
@@ -449,7 +449,7 @@ def test_pwd_cache_normalizes_domain_username(self):
                 pwd_cache=pwd_cache,
             )
             
-            assert result == "Password older than task"
+            assert result == "BAD: Password older than task"
 
     def test_pwd_cache_handles_simple_username(self):
         """Should handle username without domain prefix."""
@@ -471,7 +471,7 @@ def test_pwd_cache_handles_simple_username(self):
                 pwd_cache=pwd_cache,
             )
             
-            assert result == "Password fresh"
+            assert result == "GOOD: Password fresh"
 
     def test_pwd_cache_returns_none_when_user_not_found(self):
         """Should return None when user not in pwd_cache."""
@@ -569,7 +569,7 @@ def test_pwd_cache_skipped_when_bloodhound_has_data(self):
         )
         
         # Should use BloodHound result, not pwd_cache
-        assert result == "BloodHound analysis"
+        assert result == "HIGH: BloodHound analysis"
 
     def test_pwd_cache_used_when_bloodhound_returns_unknown(self):
         """Should fall back to pwd_cache when BloodHound returns UNKNOWN."""
@@ -595,7 +595,7 @@ def test_pwd_cache_used_when_bloodhound_returns_unknown(self):
                 pwd_cache=pwd_cache,
             )
             
-            assert result == "Cache-based analysis"
+            assert result == "GOOD: Cache-based analysis"
 
 
 class TestClassifyTaskWithPwdCache:
diff --git a/tests/test_printer.py b/tests/test_printer.py
@@ -382,7 +382,9 @@ def test_cred_validation_valid_hijackable(self, mock_resolve):
         )
 
         text = "\n".join(lines)
-        assert "[+] VALID (hijackable)" in text
+        assert "Cred Validation" in text
+        assert "VALID" in text
+        assert "INVALID" not in text
 
     @patch("taskhound.output.printer.format_runas_with_sid_resolution")
     def test_cred_validation_valid_restricted(self, mock_resolve):
@@ -405,7 +407,7 @@ def test_cred_validation_valid_restricted(self, mock_resolve):
         )
 
         text = "\n".join(lines)
-        assert "[+] VALID (restricted: logon_as_batch)" in text
+        assert "VALID (restricted: logon_as_batch)" in text
         assert "Cred Detail        : User has SeLogonAsBatchJob right" in text
 
     @patch("taskhound.output.printer.format_runas_with_sid_resolution")
@@ -427,7 +429,7 @@ def test_cred_validation_invalid(self, mock_resolve):
         )
 
         text = "\n".join(lines)
-        assert "[-] INVALID (wrong password)" in text
+        assert "INVALID (wrong password)" in text
 
     @patch("taskhound.output.printer.format_runas_with_sid_resolution")
     def test_cred_validation_blocked(self, mock_resolve):
@@ -447,7 +449,7 @@ def test_cred_validation_blocked(self, mock_resolve):
         )
 
         text = "\n".join(lines)
-        assert "[-] BLOCKED (account disabled/expired)" in text
+        assert "BLOCKED (account disabled/expired)" in text
 
     @patch("taskhound.output.printer.format_runas_with_sid_resolution")
     def test_cred_validation_unknown_with_good_password(self, mock_resolve):
@@ -469,7 +471,7 @@ def test_cred_validation_unknown_with_good_password(self, mock_resolve):
         )
 
         text = "\n".join(lines)
-        assert "[+] LIKELY VALID (task never ran, but password newer than pwdLastSet)" in text
+        assert "LIKELY VALID (task never ran, password newer than pwdLastSet)" in text
 
     @patch("taskhound.output.printer.format_runas_with_sid_resolution")
     def test_cred_validation_unknown_with_bad_password(self, mock_resolve):
@@ -491,7 +493,7 @@ def test_cred_validation_unknown_with_bad_password(self, mock_resolve):
         )
 
         text = "\n".join(lines)
-        assert "[-] LIKELY INVALID (task never ran, password older than pwdLastSet)" in text
+        assert "LIKELY INVALID (task never ran, password older than pwdLastSet)" in text
 
     @patch("taskhound.output.printer.format_runas_with_sid_resolution")
     def test_cred_validation_unknown_no_analysis(self, mock_resolve):
@@ -512,7 +514,7 @@ def test_cred_validation_unknown_no_analysis(self, mock_resolve):
         )
 
         text = "\n".join(lines)
-        assert "[?] UNKNOWN - task never ran (0x80070005)" in text
+        assert "UNKNOWN - task never ran (0x80070005)" in text
 
     @patch("taskhound.output.printer.format_runas_with_sid_resolution")
     def test_cred_validation_other_status(self, mock_resolve):
@@ -533,7 +535,7 @@ def test_cred_validation_other_status(self, mock_resolve):
         )
 
         text = "\n".join(lines)
-        assert "[?] custom_status (0x12345678)" in text
+        assert "custom_status (0x12345678)" in text
 
 
 class TestFormatBlockTaskKind:
@@ -577,7 +579,9 @@ def test_task_with_cred_validation(self, mock_resolve):
         )
 
         text = "\n".join(lines)
-        assert "[+] VALID (hijackable)" in text
+        assert "Cred Validation" in text
+        assert "VALID" in text
+        assert "INVALID" not in text
 
     @patch("taskhound.output.printer.format_runas_with_sid_resolution")
     def test_task_with_decrypted_credentials(self, mock_resolve):
