fix: more guardrail hardening

application/agents/headless_runner.py

@@ -214,6 +214,12 @@ def run_agent_headless(
         # truthy instead of reading as "ok".
         if event.get("type") == "error":
             stream_error = str(event.get("error") or "")[:500] or "unspecified"
+            if event.get("guardrail"):
+                # Same rule as the streaming route: a blocked turn must not
+                # record what was blocked. A scheduled run has no client to
+                # retract from, so the stored result is all there is.
+                answer_full = ""
+                thought = ""
             continue
         # A workflow's work is its nodes: its tool calls stay in the engine's
         # execution log and its node agents own their LLMs, so neither

application/api/answer/routes/base.py

@@ -693,7 +693,13 @@ class BaseAnswerResource:
                             # the wire cannot be recalled, but the persisted
                             # message must not keep them — otherwise reloading
                             # the page redisplays exactly what was just blocked.
+                            # ``thought`` counts: a reasoning model states its
+                            # intent before acting on it, so the trace is where
+                            # the blocked material appears first. The client
+                            # clears it live, so leaving it here would surface
+                            # it only on reload.
                             response_full = error_text
+                            thought = ""
                             structured_chunks.clear()
                             is_structured = False
                             query_metadata["guardrail"] = guardrail_meta

application/guardrails/config.py

@@ -7,14 +7,28 @@ all-defaults so a malformed row never breaks a stream).
 
 from __future__ import annotations
 
+import logging
 from typing import Any, Dict, List, Optional
 
 from pydantic import BaseModel, ConfigDict, field_validator, model_validator
 
 from application.guardrails.types import ACTIONS_BY_STAGE, Action, Stage
 
+logger = logging.getLogger(__name__)
+
 DEFAULT_BLOCK_MESSAGE = "Sorry, I can't help with that request."
 
+
+def _reason(exc: Exception) -> str:
+    """The operator-readable half of a pydantic validation error."""
+    errors = getattr(exc, "errors", None)
+    if callable(errors):
+        try:
+            return str(errors()[0].get("msg", exc)).replace("Value error, ", "")
+        except Exception:
+            pass
+    return str(exc)
+
 MODES = ("monitor_only", "scan_all")
 
 
@@ -141,13 +155,55 @@ class GuardrailsConfig(BaseModel):
 
     @classmethod
     def parse(cls, raw: Optional[dict]) -> "GuardrailsConfig":
-        """Lenient read: all-defaults (disabled) for empty or invalid input."""
+        """Lenient read: never raises, so a bad row can't break a stream.
+
+        A control that stopped validating — its check disallowed by
+        ``GUARDRAILS_CHECKS_ENABLED``, or renamed/removed in an upgrade — is
+        dropped on its own. Discarding the whole config instead turned one
+        stale control into "this agent has no guardrails at all", so an
+        operator *tightening* the allowlist silently stripped every remaining
+        control from every affected agent.
+        """
         if not raw or not isinstance(raw, dict):
             return cls()
         try:
             return cls.model_validate(raw)
         except Exception:
+            return cls._salvage(raw)
+
+    @classmethod
+    def _salvage(cls, raw: dict) -> "GuardrailsConfig":
+        """Re-validate control by control, keeping the ones that still pass."""
+        rest = {key: value for key, value in raw.items() if key != "controls"}
+        entries = raw.get("controls")
+        kept: List[Any] = []
+        dropped: List[str] = []
+        if isinstance(entries, list):
+            for entry in entries:
+                try:
+                    GuardrailControl.model_validate(entry)
+                except Exception as exc:
+                    label = entry.get("check") if isinstance(entry, dict) else "?"
+                    stage = entry.get("stage") if isinstance(entry, dict) else "?"
+                    dropped.append(f"{label}:{stage} — {_reason(exc)}")
+                    continue
+                kept.append(entry)
+        try:
+            parsed = cls.model_validate({**rest, "controls": kept})
+        except Exception:
+            logger.warning(
+                "Agent guardrails config is unusable and is being ignored; "
+                "this agent runs unguarded until it is re-saved"
+            )
             return cls()
+        if dropped:
+            logger.warning(
+                "Dropped %d unusable guardrail control(s); %d still active: %s",
+                len(dropped),
+                len(kept),
+                "; ".join(dropped),
+            )
+        return parsed
 
 
 class AgentConfig(BaseModel):

application/guardrails/engine.py

@@ -86,7 +86,7 @@ class GuardrailEngine:
         ``controls`` narrows the run to a subset of the stage's controls; the
         streaming guard uses it to defer complete-text checks to the final scan.
         """
-        decision = StageDecision(stage=stage, text=text)
+        decision = StageDecision(stage=stage, text=text, original_text=text)
         if controls is None:
             controls = self.config.controls_for(stage)
         if not controls:

application/guardrails/runtime.py

@@ -197,7 +197,9 @@ class GuardrailRecorder:
                         "score": verdict.outcome.score,
                         "match_count": len(verdict.outcome.spans),
                         "matched_value": (
-                            self._sample(decision.text, verdict) if store_text else None
+                            self._sample(decision.scanned_text, verdict)
+                            if store_text
+                            else None
                         ),
                         "detail": verdict.outcome.detail or verdict.outcome.error,
                     }

application/guardrails/stream.py

@@ -22,7 +22,13 @@ from typing import List, Optional
 
 from application.guardrails.engine import GuardrailEngine
 from application.guardrails.guardrail_creator import GuardrailCreator
-from application.guardrails.types import Stage, StageDecision
+from application.guardrails.types import (
+    Action,
+    Span,
+    Stage,
+    StageDecision,
+    apply_spans,
+)
 
 DEFAULT_LOOKBACK = 128
 DEFAULT_SEGMENT_CHARS = 400
@@ -242,22 +248,54 @@ class StreamingOutputGuard:
                 decisions=[decision],
             )
 
-        return self._split(decision.text, force=force, final=final, decision=decision)
+        return self._split(combined, force=force, final=final, decision=decision)
+
+    @staticmethod
+    def _redact_spans(decision: Optional[StageDecision]) -> List[Span]:
+        """Spans a redact control asked to mask, in scan-buffer coordinates."""
+        if decision is None:
+            return []
+        spans: List[Span] = []
+        for verdict in decision.verdicts:
+            if verdict.action is Action.REDACT and verdict.outcome.triggered:
+                spans.extend(verdict.outcome.spans)
+        return spans
+
+    @staticmethod
+    def _safe_release(emit_end: int, spans: List[Span]) -> int:
+        """Pull the release point back so no masked span is cut in half.
+
+        Emitting the front of a match and holding the rest leaks it: the held
+        remainder is a truncated tail that no longer satisfies the pattern, so
+        nothing re-matches it and it goes out in the clear.
+        """
+        while True:
+            crossing = [s.start for s in spans if s.start < emit_end < s.end]
+            if not crossing:
+                return emit_end
+            emit_end = min(crossing)
 
     def _split(
-        self, scanned: str, force: bool, final: bool, decision: Optional[StageDecision]
+        self, raw: str, force: bool, final: bool, decision: Optional[StageDecision]
     ) -> StreamChunk:
+        spans = self._redact_spans(decision)
         if final:
-            emit_end = len(scanned)
+            emit_end = len(raw)
         else:
-            emit_end = self._release_point(scanned)
+            emit_end = self._release_point(raw)
             if force and emit_end == 0:
                 # Over the hold ceiling with no boundary in sight: release all
                 # but the lookback tail so the stream cannot stall forever.
-                emit_end = max(0, len(scanned) - self.lookback)
+                emit_end = max(0, len(raw) - self.lookback)
+            emit_end = self._safe_release(emit_end, spans)
 
-        self._held = scanned[emit_end:]
-        emitted = scanned[:emit_end]
+        # The tail is held *unredacted*. Masking before holding would freeze a
+        # partial match: a detector firing on a prefix of a value still arriving
+        # bakes the mask in, and the characters that follow append after it and
+        # are never re-scanned. Holding raw is safe because the tail is
+        # re-scanned in full on every subsequent round.
+        self._held = raw[emit_end:]
+        emitted = apply_spans(raw[:emit_end], [s for s in spans if s.end <= emit_end])
         if self._full is not None:
             self._full += emitted
         return StreamChunk(

application/guardrails/types.py

@@ -126,6 +126,15 @@ class StageDecision:
     redacted: bool = False
     block_message: Optional[str] = None
     verdicts: List[ControlVerdict] = field(default_factory=list)
+    # The text as it was scanned, before any redaction. Span offsets belong to
+    # this string; once ``text`` has been masked they no longer index it, so
+    # anything resolving a span back to its matched value must read this.
+    original_text: Optional[str] = None
+
+    @property
+    def scanned_text(self) -> str:
+        """The string span offsets are valid against."""
+        return self.original_text if self.original_text is not None else self.text
 
     @property
     def triggered(self) -> List[ControlVerdict]:

frontend/src/agents/NewAgent.tsx

@@ -210,6 +210,12 @@ export default function NewAgent({ mode }: { mode: 'new' | 'edit' | 'draft' }) {
     return jsonSchemaText.trim() !== '' && !jsonSchemaValid;
   };
 
+  // Draft saves post the same `config` blob and hit the same server-side
+  // validation as publish, so they need the same gate — otherwise an
+  // incomplete control 400s the draft behind a message that names no field.
+  const isDraftBlocked = () =>
+    isJsonSchemaInvalid() || guardrailsIncomplete(agent.config?.guardrails);
+
   // Resolve a selected source id to its display name. Prefer the caller's own
   // source list; fall back to the owner-resolved name embedded in the agent
   // payload (source_details) so a team member viewing a shared agent sees the
@@ -885,10 +891,10 @@ export default function NewAgent({ mode }: { mode: 'new' | 'edit' | 'draft' }) {
           {modeConfig[effectiveMode].showSaveDraft && (
             <Button
               type="button"
-              disabled={isJsonSchemaInvalid()}
+              disabled={isDraftBlocked()}
               onClick={handleSaveDraft}
               className={`border-primary text-primary hover:bg-primary/90 min-w-28 rounded-3xl border border-solid bg-transparent px-5 whitespace-nowrap hover:text-white ${
-                isJsonSchemaInvalid() ? 'disabled:opacity-30' : ''
+                isDraftBlocked() ? 'disabled:opacity-30' : ''
               }`}
             >
               <span className="flex items-center justify-center transition-all duration-200">

frontend/src/agents/components/GuardrailsSection.tsx

@@ -68,10 +68,14 @@ export function controlNeedsSetup(control: GuardrailControl): boolean {
  * True when saving would 400. The form gates Save on this so a single click on
  * a stage chip can't leave the whole agent — name, model and all — unsavable
  * behind an error naming an array index.
+ *
+ * Deliberately ignores `config.enabled`: the backend validates every control
+ * whether or not guardrails are switched on, so gating on `enabled` here let
+ * an incomplete control through the moment the user toggled guardrails off.
  */
 export function guardrailsIncomplete(config?: GuardrailsConfig): boolean {
-  if (!config?.enabled) return false;
-  return config.controls.some(controlNeedsSetup);
+  if (!config) return false;
+  return (config.controls ?? []).some(controlNeedsSetup);
 }
 
 function latencyLabel(ms: number): string {

frontend/src/conversation/conversationSlice.ts

@@ -754,6 +754,18 @@ export const submitToolActions = createAsyncThunk<
             message: data.notice ?? '',
           }),
         );
+      } else if (data.type === 'guardrail') {
+        // A resumed turn is still the same turn: the output guard runs on the
+        // continuation too, so this path needs the same retraction as the
+        // initial stream or the blocked text stays on screen until reload.
+        if (data.retract) {
+          dispatch(
+            conversationSlice.actions.retractResponse({
+              conversationId,
+              index: targetIndex,
+            }),
+          );
+        }
       } else if (data.type === 'error') {
         dispatch(conversationSlice.actions.setStatus('failed'));
         dispatch(

frontend/src/conversation/sharedConversationSlice.ts

@@ -79,6 +79,17 @@ export const fetchSharedAnswer = createAsyncThunk<Answer, { question: string }>(
                   query: { tool_calls: data.tool_calls },
                 }),
               );
+            } else if (data.type === 'guardrail') {
+              // Shared conversations stream through the same route and so hit
+              // the same output guard. Without this the event fell into the
+              // final `else` and was read as an answer chunk.
+              if (data.retract) {
+                dispatch(
+                  sharedConversationSlice.actions.retractResponse({
+                    index: state.sharedConversation.queries.length - 1,
+                  }),
+                );
+              }
             } else if (data.type === 'error') {
               // set status to 'failed'
               dispatch(sharedConversationSlice.actions.setStatus('failed'));
@@ -242,6 +253,16 @@ export const sharedConversationSlice = createSlice({
       const { index, message } = action.payload;
       state.queries[index].error = message;
     },
+    retractResponse(state, action: PayloadAction<{ index: number }>) {
+      // A guardrail tripped after tokens were already rendered. The backend
+      // has replaced the persisted message with the block message, so drop the
+      // partial answer here too — otherwise the leaked text stays on screen
+      // until reload and contradicts what is in the database.
+      const { index } = action.payload;
+      if (!state.queries[index]) return;
+      state.queries[index].response = '';
+      state.queries[index].thought = '';
+    },
     saveToLocalStorage(state) {
       const previousQueriesStr = localStorage.getItem(state.identifier);
       previousQueriesStr
@@ -289,6 +310,7 @@ export const {
   addQuery,
   saveToLocalStorage,
   updateStreamingSource,
+  retractResponse,
 } = sharedConversationSlice.actions;
 
 export const selectStatus = (state: RootState) =>

tests/guardrails/test_config_salvage.py

@@ -0,0 +1,103 @@
+"""A stale control must not take the rest of an agent's guardrails with it.
+
+Also pins ``GUARDRAILS_CHECKS_ENABLED`` to the registry keys. The equivalent
+setting for retrievers drifted from its registry once already, and the failure
+is silent both times: an unmatched value is filtered out, not rejected.
+"""
+
+from __future__ import annotations
+
+import logging
+
+import pytest
+
+from application.core.settings import settings
+from application.guardrails.config import AgentConfig, GuardrailsConfig
+from application.guardrails.guardrail_creator import GuardrailCreator
+from application.guardrails.runtime import resolve_config
+
+TWO_CONTROLS = {
+    "enabled": True,
+    "mode": "scan_all",
+    "controls": [
+        {"check": "secrets", "stage": "output", "action": "redact"},
+        {"check": "pii", "stage": "output", "action": "redact"},
+    ],
+}
+
+
+def _checks(config):
+    return [c.check for c in config.controls]
+
+
+@pytest.fixture
+def narrowed(monkeypatch):
+    """Allow only ``pii``, as an operator tightening the allowlist would."""
+    monkeypatch.setattr(settings, "GUARDRAILS_CHECKS_ENABLED", ["pii"])
+    monkeypatch.setattr(GuardrailCreator, "_bootstrapped", False)
+    yield
+
+
+class TestAllowlistMatchesRegistry:
+    def test_default_allowlist_is_empty_meaning_everything(self):
+        assert settings.GUARDRAILS_CHECKS_ENABLED == []
+        assert GuardrailCreator.enabled_keys() == sorted(GuardrailCreator.checks)
+
+    def test_every_configured_value_is_a_registry_key(self):
+        """Guards against the drift that silently disabled a retriever."""
+        configured = settings.GUARDRAILS_CHECKS_ENABLED or []
+        GuardrailCreator._ensure_builtin()
+        unknown = set(configured) - set(GuardrailCreator.checks)
+        assert not unknown, f"not registry keys: {sorted(unknown)}"
+
+    def test_catalog_only_exposes_enabled_keys(self, narrowed):
+        assert [entry["name"] for entry in GuardrailCreator.catalog()] == ["pii"]
+
+    def test_a_typo_is_filtered_not_rejected(self, monkeypatch):
+        """Documents the sharp edge the test above exists to catch."""
+        monkeypatch.setattr(settings, "GUARDRAILS_CHECKS_ENABLED", ["pii", "secrets_"])
+        monkeypatch.setattr(GuardrailCreator, "_bootstrapped", False)
+        assert GuardrailCreator.enabled_keys() == ["pii"]
+
+
+class TestSalvage:
+    def test_disallowed_control_is_dropped_alone(self, narrowed):
+        parsed = GuardrailsConfig.parse(TWO_CONTROLS)
+        assert parsed.enabled is True
+        assert _checks(parsed) == ["pii"]
+
+    def test_agent_config_wrapper_salvages_too(self, narrowed):
+        parsed = AgentConfig.parse({"guardrails": TWO_CONTROLS}).guardrails
+        assert _checks(parsed) == ["pii"]
+
+    def test_resolve_config_end_to_end(self, narrowed):
+        assert _checks(resolve_config({"guardrails": TWO_CONTROLS})) == ["pii"]
+
+    def test_unknown_check_from_an_upgrade_is_dropped_alone(self):
+        raw = {
+            "enabled": True,
+            "mode": "scan_all",
+            "controls": [
+                {"check": "moderation", "stage": "output", "action": "flag"},
+                {"check": "secrets", "stage": "output", "action": "redact"},
+            ],
+        }
+        assert _checks(GuardrailsConfig.parse(raw)) == ["secrets"]
+
+    def test_mode_preserved_while_salvaging(self, narrowed):
+        assert GuardrailsConfig.parse(TWO_CONTROLS).mode == "scan_all"
+
+    def test_drop_is_logged_with_the_reason(self, narrowed, caplog):
+        with caplog.at_level(logging.WARNING, logger="application.guardrails.config"):
+            GuardrailsConfig.parse(TWO_CONTROLS)
+        assert "secrets:output" in caplog.text
+        assert "not enabled on this instance" in caplog.text
+
+    def test_unusable_config_still_degrades_to_disabled(self, caplog):
+        with caplog.at_level(logging.WARNING, logger="application.guardrails.config"):
+            parsed = GuardrailsConfig.parse({"enabled": True, "mode": "not_a_mode"})
+        assert parsed.enabled is False
+        assert "unusable" in caplog.text
+
+    def test_valid_config_is_untouched(self):
+        assert _checks(GuardrailsConfig.parse(TWO_CONTROLS)) == ["secrets", "pii"]

tests/guardrails/test_redaction_boundaries.py

@@ -0,0 +1,143 @@
+"""Redaction must survive chunk boundaries and must not corrupt the journal.
+
+Two bugs shared one cause: span offsets are computed against the text as
+scanned, so anything that consumes them after ``_reduce`` has masked the text
+is indexing a string that no longer exists.
+"""
+
+from __future__ import annotations
+
+import pytest
+
+from application.core.settings import settings
+from application.guardrails.config import GuardrailsConfig
+from application.guardrails.engine import GuardrailEngine
+from application.guardrails.runtime import GuardrailRecorder
+from application.guardrails.stream import StreamingOutputGuard
+from application.guardrails.types import Stage
+
+# Long enough that the guard splits mid-stream instead of scanning once at
+# flush; below the window the bug is invisible, which is why it shipped.
+PAD = ("The quick brown fox jumps over the lazy dog near the riverbank. " * 45)[:2700]
+
+SECRETS = {
+    # Hyphenated on purpose: these satisfy SECRET_PATTERNS but not GitHub's
+    # push-protection detectors, which need long unbroken alphanumeric runs.
+    "openai": "sk-not-a-real-openai-key-for-tests-only",
+    "slack": "xoxb-not-a-real-slack-token-for-tests",
+    "anthropic": "sk-ant-api03XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX",
+    "jwt": "eyJhbGciOiJIUzI1NiJ9.eyJzdWIiOiIxMjM0NTY3ODkwIn0.dBjftJeZ4CVPmB92K27uhbUJU1p1r_wW1gFWFOEjXk",
+    "github_long": "ghp_" + "A" * 44,
+}
+
+
+def _guard(check, action="redact", **settings_over):
+    config = GuardrailsConfig.model_validate(
+        {
+            "enabled": True,
+            "mode": "scan_all",
+            "controls": [
+                {
+                    "check": check,
+                    "stage": "output",
+                    "action": action,
+                    "settings": settings_over,
+                }
+            ],
+        }
+    )
+    return StreamingOutputGuard(GuardrailEngine(config))
+
+
+def _stream(guard, text, chunk):
+    out = []
+    for i in range(0, len(text), chunk):
+        out.append(guard.feed(text[i : i + chunk]).emit)
+    out.append(guard.flush().emit)
+    return "".join(out)
+
+
+class TestStraddlingMatch:
+    """A value whose regex is satisfied by a prefix still being streamed."""
+
+    @pytest.mark.parametrize("name", sorted(SECRETS))
+    def test_secret_longer_than_its_pattern_minimum_never_leaks(self, name):
+        secret = SECRETS[name]
+        emitted = _stream(_guard("secrets"), f"{PAD} key {secret} end.", chunk=4)
+        assert "[REDACTED]" in emitted
+        assert secret not in emitted
+        # The tail is what used to survive: the detector matched a prefix, the
+        # mask was written into the held buffer, and the rest appended after it.
+        assert emitted.endswith("[REDACTED] end.")
+
+    @pytest.mark.parametrize("chunk", [1, 3, 4, 17, 128, 1000])
+    def test_result_is_independent_of_chunk_size(self, chunk):
+        secret = SECRETS["openai"]
+        emitted = _stream(_guard("secrets"), f"{PAD} key {secret} end.", chunk)
+        assert secret not in emitted
+        assert "not-a-real" not in emitted
+
+    @pytest.mark.parametrize("pad", [200, 250, 400, 950])
+    def test_email_at_the_window_boundary(self, pad):
+        text = ("Padding sentence here. " * 60)[:pad] + " Write to a@example.com now."
+        emitted = _stream(_guard("pii", entities=["EMAIL"]), text, chunk=4)
+        assert "@" not in emitted
+        assert emitted.endswith("Write to [EMAIL] now.")
+
+    @pytest.mark.parametrize(
+        "card", ["4575513137353792", "4111111111111111", "5500005555555559"]
+    )
+    def test_credit_card_trailing_digits_never_leak(self, card):
+        emitted = _stream(
+            _guard("pii", entities=["CREDIT_CARD"]), f"{PAD} card {card} end.", chunk=4
+        )
+        assert card not in emitted
+        assert emitted.endswith("[CREDIT_CARD] end.")
+
+
+class TestNoCollateralDamage:
+    @pytest.mark.parametrize("chunk", [1, 4, 37, 500])
+    def test_clean_text_passes_through_byte_identical(self, chunk):
+        clean = PAD + " nothing sensitive here at all. end."
+        assert _stream(_guard("secrets"), clean, chunk) == clean
+
+    def test_held_tail_is_kept_unredacted(self):
+        """The buffer is re-scanned every round, so masking it early is loss."""
+        guard = _guard("secrets")
+        guard.feed(PAD + " key sk-not-a-real-openai-key")
+        assert "[REDACTED]" not in guard.pending
+
+
+class TestAuditSpanOffsets:
+    @pytest.fixture
+    def store_text(self, monkeypatch):
+        monkeypatch.setattr(settings, "GUARDRAILS_STORE_SCANNED_TEXT", True)
+
+    @staticmethod
+    def _rows(action):
+        rows = []
+        recorder = GuardrailRecorder(mode="scan_all")
+        recorder._rows = rows
+        config = GuardrailsConfig.model_validate(
+            {
+                "enabled": True,
+                "mode": "scan_all",
+                "controls": [{"check": "pii", "stage": "output", "action": action}],
+            }
+        )
+        engine = GuardrailEngine(config, recorder=recorder)
+        engine.evaluate(
+            "Hello world, please contact alice@example.com right away "
+            "and also bob@example.com thanks",
+            Stage.OUTPUT,
+        )
+        return rows
+
+    def test_redact_records_the_match_not_the_masked_text(self, store_text):
+        assert self._rows("redact")[0]["matched_value"] == "alice@example.com"
+
+    def test_flag_is_unaffected(self, store_text):
+        assert self._rows("flag")[0]["matched_value"] == "alice@example.com"
+
+    def test_nothing_is_stored_when_the_setting_is_off(self):
+        assert self._rows("redact")[0]["matched_value"] is None