overseer: scrub secrets in AdvisorParseError messages

shared/egg_overseer/advisor.py

@@ -259,16 +259,30 @@ async def _default_runner(p: str, m: str) -> str:
         if payload is None:
             # No `{` ever found → no useful inner exception; otherwise
             # chain the most recent ``raw_decode`` failure for context.
+            # Scrub before raising: ``raw`` is the unparsed model output
+            # and ends up in stderr via ``cmd_overseer_consult_advisor``.
+            # NOTE: ``__cause__`` (last_exc) preserves the original
+            # ``JSONDecodeError`` whose ``str()`` can echo input values.
+            # Safe today because no caller renders the chained traceback
+            # — if you add ``traceback.format_exc()`` or
+            # ``logger.exception()`` upstream, scrub there too.
             raise AdvisorParseError(
-                f"consult_advisor: SDK response is not valid JSON: {raw!r}"
+                scrub_secrets(f"consult_advisor: SDK response is not valid JSON: {raw!r}")
             ) from last_exc
 
     try:
         verdict = AdvisorVerdict.model_validate(payload)
     except Exception as exc:
+        # Scrub: ``payload`` and the pydantic error both echo input
+        # values that may include credentials the model parroted back.
+        # Same ``__cause__`` caveat as the JSON-decode path above:
+        # ``ValidationError`` carries unscrubbed input; safe only while
+        # callers don't render the chained traceback.
         raise AdvisorParseError(
-            f"consult_advisor: SDK response failed AdvisorVerdict "
-            f"validation: {exc}; payload={payload!r}"
+            scrub_secrets(
+                f"consult_advisor: SDK response failed AdvisorVerdict "
+                f"validation: {exc}; payload={payload!r}"
+            )
         ) from exc
 
     # Defense-in-depth: scrub the body before it leaves this function.

shared/tests/test_overseer_advisor.py

@@ -339,6 +339,47 @@ def test_schema_failure_raises_parse_error(self) -> None:
                 )
             )
 
+    def test_invalid_json_error_message_is_scrubbed(self) -> None:
+        # Raw model output is embedded in the AdvisorParseError message
+        # and ends up in stderr via cmd_overseer_consult_advisor. If the
+        # model parrots a credential in its prose, the error must not
+        # surface it verbatim.
+        runner = self._runner_returning(f"oops, leaked {_GH_PAT} not json")
+        with pytest.raises(AdvisorParseError) as excinfo:
+            asyncio.run(
+                consult_advisor(
+                    classification={},
+                    health_alerts=[],
+                    progress_events=[],
+                    recent_log_lines=[],
+                    _agent_runner=runner,
+                )
+            )
+        message = str(excinfo.value)
+        assert _GH_PAT not in message
+        assert "[REDACTED:gh-pat]" in message
+
+    def test_validation_error_message_is_scrubbed(self) -> None:
+        # Same concern on the schema-validation path: the payload repr
+        # and pydantic error both echo input values back into the
+        # message string.
+        runner = self._runner_returning(
+            {"decision": "alert", "alert_summary": f"saw token {_GH_PAT}"}
+        )
+        with pytest.raises(AdvisorParseError) as excinfo:
+            asyncio.run(
+                consult_advisor(
+                    classification={},
+                    health_alerts=[],
+                    progress_events=[],
+                    recent_log_lines=[],
+                    _agent_runner=runner,
+                )
+            )
+        message = str(excinfo.value)
+        assert _GH_PAT not in message
+        assert "[REDACTED:gh-pat]" in message
+
     def test_default_model_used_when_config_none(self) -> None:
         seen: dict[str, str] = {}