fix(inspection): stage 3 content classification was dead when AGT is installed

src/cmcp_runtime/inspection/pipeline.py

@@ -222,8 +222,16 @@ def _stage4_injection_detection(
                     injection_score=score,
                 )
             return StageResult(stage="injection", decision="allow")
-        except Exception:  # nosec B110
-            pass  # Fall through to regex
+        except Exception:
+            # Falling through to the local patterns is the right behaviour, but
+            # doing it silently means a broken AGT detector looks identical to a
+            # working one while the weaker starter set is what actually ran.
+            _log.warning(
+                "AGT PromptInjectionDetector.detect() failed; "
+                "falling back to local patterns v%s",
+                _PATTERNS_VERSION,
+                exc_info=True,
+            )
 
     # Fallback: regex patterns
     patterns = custom_patterns or _COMPILED_PATTERNS
@@ -288,7 +296,7 @@ def _classify_sensitivity(
 
     1. catalog_entry.sensitivity_level: always applied
     2. field-level x-sensitivity annotations in output_schema properties
-    3. pattern matching on response content (AGT CredentialRedactor or regex fallback)
+    3. pattern matching on response content (AGT CredentialRedactor and local patterns)
     """
     tags: list[str] = []
 
@@ -312,22 +320,47 @@ def _classify_sensitivity(
         except (json.JSONDecodeError, ValueError):
             pass
 
-    # Source 3: content pattern matching
+    # Source 3: content pattern matching.
+    #
+    # AGT and the local patterns both run, rather than AGT displacing them. Two
+    # reasons. First, AGT's CredentialRedactor does not redact a secret at all
+    # when a suffix is glued to it (microsoft/agent-governance-toolkit#3494:
+    # `AKIA..._old` passes through whole), so treating it as sufficient leaves a
+    # live key untagged. Second, an either/or meant any breakage on the AGT path
+    # silently disabled content classification entirely instead of degrading to
+    # the local patterns.
     if response_text:
+        _SENSITIVE = ("pii", "confidential", "hipaa_phi", "mnpi")
+
         if _AGT_AVAILABLE and _agt_redactor is not None:
-            try:
-                matches = _agt_redactor.find_credentials(response_text)
-                if matches and not any(t in tags for t in ("pii", "confidential", "hipaa_phi", "mnpi")):
-                    tags.append("pii")
-            except Exception:  # nosec B110
-                pass
-        else:
-            # Regex fallback for when AGT is unavailable
-            for pattern, tag in _PII_PATTERNS:
-                if pattern.search(response_text) and tag not in tags:
-                    tags.append(tag)
-                    if len(tags) >= 4:  # cap scan at 4 distinct tags
-                        break
+            # find_matches covers secrets, find_pii_matches covers PII spans.
+            # Both are classmethods on CredentialRedactor. A previous version
+            # called `find_credentials`, which has never existed on that class
+            # in any agt-core release, so this whole branch raised AttributeError
+            # into the handler below and contributed nothing.
+            for method in ("find_matches", "find_pii_matches"):
+                finder = getattr(_agt_redactor, method, None)
+                if finder is None:
+                    # Narrow and loud: an AGT API that moved should surface as a
+                    # degraded stage, not as silent unconditional passing.
+                    _log.warning(
+                        "AGT CredentialRedactor has no %s(); "
+                        "falling back to local patterns for content classification",
+                        method,
+                    )
+                    continue
+                try:
+                    if finder(response_text) and not any(t in tags for t in _SENSITIVE):
+                        tags.append("pii")
+                except Exception:  # nosec B110 - never let detection break the pipeline
+                    _log.warning("AGT CredentialRedactor.%s() failed", method, exc_info=True)
+
+        # Local patterns always run, as a second pass rather than a fallback.
+        for pattern, tag in _PII_PATTERNS:
+            if len(tags) >= 4:  # cap scan at 4 distinct tags
+                break
+            if pattern.search(response_text) and tag not in tags:
+                tags.append(tag)
 
     return tags
 
@@ -339,7 +372,7 @@ class SensitivityClassificationStage:
     Applies three classification sources in order:
     1. catalog_entry.sensitivity_level annotation
     2. x-sensitivity field-level tags in output_schema properties
-    3. Content pattern matching (AGT CredentialRedactor or regex fallback)
+    3. Content pattern matching (AGT CredentialRedactor and local patterns)
     """
 
     def run(
@@ -398,13 +431,35 @@ def __init__(
         self._agt_redactor: Any = None
         self._agt_response_scanner: Any = None
         if _AGT_AVAILABLE:
+            # Constructed independently. Sharing one try block meant a failure in
+            # the first component skipped the other two, so a single upstream API
+            # change disabled three security components at once, silently.
             try:
                 _cfg = DetectionConfig(sensitivity=injection_sensitivity)
                 self._agt_injection_detector = PromptInjectionDetector(config=_cfg)
+            except Exception:
+                _log.warning(
+                    "AGT PromptInjectionDetector unavailable; "
+                    "stage 4 falls back to local patterns v%s",
+                    _PATTERNS_VERSION,
+                    exc_info=True,
+                )
+            try:
                 self._agt_redactor = CredentialRedactor()
+            except Exception:
+                _log.warning(
+                    "AGT CredentialRedactor unavailable; "
+                    "stage 3 falls back to local patterns",
+                    exc_info=True,
+                )
+            try:
                 self._agt_response_scanner = AGTResponseScanner()
-            except Exception:  # nosec B110
-                pass
+            except Exception:
+                _log.warning(
+                    "AGT MCPResponseScanner unavailable; "
+                    "stage 4 loses MCP-specific threat detection",
+                    exc_info=True,
+                )
 
     def run(
         self,
@@ -518,8 +573,17 @@ def run(
                 injection_scanner = "timeout"
                 stage_results["injection"] = "deny"
                 agt_mcp_denied = True
-            except Exception:  # nosec B110
-                pass
+            except Exception:
+                # Unlike the timeout above this does not deny, because a scanner
+                # that errored has produced no verdict either way and stage 4's
+                # own detection still runs below. It must not be silent though:
+                # MCP-specific threat coverage is gone for this response.
+                _log.warning(
+                    "AGT MCPResponseScanner.scan_response() failed for %s; "
+                    "no MCP-specific threat coverage on this response",
+                    catalog_entry.tool_name,
+                    exc_info=True,
+                )
 
         # INJECT-002: wrap AGT PromptInjectionDetector with the same timeout bound.
         def _run_s4() -> StageResult:

tests/unit/test_stage3_sensitivity.py

@@ -160,3 +160,114 @@ def test_no_duplicate_pii_from_catalog_and_field():
     with patch("cmcp_runtime.inspection.pipeline._AGT_AVAILABLE", False):
         result = stage.run({"ssn": "123-45-6789"}, _make_entry("pii", output_schema=schema))
     assert result.sensitivity_tags.count("pii") == 1
+
+
+# ── Source 3 with AGT present: local patterns must still run (#476) ───────────
+#
+# The previous implementation called `_agt_redactor.find_credentials()`, a method
+# that has never existed on agt-core's CredentialRedactor (verified against the
+# pinned 4.1.0 wheel: the real names are find_matches / find_pii_matches). The
+# call raised AttributeError into a bare `except Exception: pass`, and because
+# the local patterns sat in the `else` branch of that check they never ran
+# either. With AGT installed, content-based classification was dead.
+
+class _RedactorMissingMethod:
+    """An agt-core CredentialRedactor without the method the caller expects."""
+
+
+class _RedactorRaising:
+    @staticmethod
+    def find_matches(_value: str) -> list[str]:
+        raise RuntimeError("upstream blew up")
+
+    @staticmethod
+    def find_pii_matches(_value: str) -> list[str]:
+        raise RuntimeError("upstream blew up")
+
+
+class _RedactorSuffixBlind:
+    """Reproduces microsoft/agent-governance-toolkit#3494: a secret with a
+    suffix glued to it is not matched at all, so AGT alone calls it clean."""
+
+    @staticmethod
+    def find_matches(value: str) -> list[str]:
+        return ["AKIA"] if "AKIAIOSFODNN7EXAMPLE" in value and "_old" not in value else []
+
+    @staticmethod
+    def find_pii_matches(_value: str) -> list[str]:
+        return []
+
+
+def test_local_patterns_still_run_when_agt_lacks_the_method():
+    stage = SensitivityClassificationStage()
+    with patch("cmcp_runtime.inspection.pipeline._AGT_AVAILABLE", True):
+        result = stage.run(
+            {"data": "SSN is 123-45-6789"}, _make_entry(), _RedactorMissingMethod()
+        )
+    assert "pii" in result.sensitivity_tags
+
+
+def test_local_patterns_still_run_when_agt_raises():
+    stage = SensitivityClassificationStage()
+    with patch("cmcp_runtime.inspection.pipeline._AGT_AVAILABLE", True):
+        result = stage.run(
+            {"data": "SSN is 123-45-6789"}, _make_entry(), _RedactorRaising()
+        )
+    assert "pii" in result.sensitivity_tags
+
+
+def test_suffixed_secret_still_tagged_despite_agt_3494():
+    stage = SensitivityClassificationStage()
+    blind = _RedactorSuffixBlind()
+    with patch("cmcp_runtime.inspection.pipeline._AGT_AVAILABLE", True):
+        clean = stage.run({"k": "AKIAIOSFODNN7EXAMPLE x@y.com"}, _make_entry(), blind)
+        suffixed = stage.run({"k": "AKIAIOSFODNN7EXAMPLE_old x@y.com"}, _make_entry(), blind)
+    # AGT catches the bare key; only the local pass catches the suffixed one.
+    assert "pii" in clean.sensitivity_tags
+    assert "pii" in suffixed.sensitivity_tags
+
+
+# ── AGT component construction is independent (#476) ──────────────────────────
+#
+# All three components were built in one try block, so a failure constructing
+# the first left the other two unbuilt and all three silently None. One upstream
+# API change disabled three security components at once, with no log line.
+
+def test_one_failing_agt_component_does_not_disable_the_others():
+    from cmcp_runtime.inspection.pipeline import InspectionPipeline
+
+    def _boom(*_a: object, **_k: object) -> None:
+        raise RuntimeError("upstream API changed")
+
+    with (
+        patch("cmcp_runtime.inspection.pipeline._AGT_AVAILABLE", True),
+        patch("cmcp_runtime.inspection.pipeline.DetectionConfig", _boom),
+        patch("cmcp_runtime.inspection.pipeline.CredentialRedactor", lambda: "redactor"),
+        patch("cmcp_runtime.inspection.pipeline.AGTResponseScanner", lambda: "scanner"),
+    ):
+        pipeline = InspectionPipeline()
+
+    # The detector failed, but the other two must still be constructed.
+    assert pipeline._agt_injection_detector is None
+    assert pipeline._agt_redactor == "redactor"
+    assert pipeline._agt_response_scanner == "scanner"
+
+
+def test_failing_agt_component_is_logged_not_swallowed(caplog: Any) -> None:
+    import logging
+
+    from cmcp_runtime.inspection.pipeline import InspectionPipeline
+
+    def _boom(*_a: object, **_k: object) -> None:
+        raise RuntimeError("upstream API changed")
+
+    with (
+        caplog.at_level(logging.WARNING, logger="cmcp_runtime.inspection.pipeline"),
+        patch("cmcp_runtime.inspection.pipeline._AGT_AVAILABLE", True),
+        patch("cmcp_runtime.inspection.pipeline.DetectionConfig", _boom),
+        patch("cmcp_runtime.inspection.pipeline.CredentialRedactor", lambda: "redactor"),
+        patch("cmcp_runtime.inspection.pipeline.AGTResponseScanner", lambda: "scanner"),
+    ):
+        InspectionPipeline()
+
+    assert any("PromptInjectionDetector unavailable" in r.message for r in caplog.records)