From 3adc09af5d9ee114b255484f840c45781848b490 Mon Sep 17 00:00:00 2001 From: Alex Date: Tue, 11 Aug 2026 15:08:59 +0100 Subject: [PATCH] fix: more guardrail hardening --- application/agents/headless_runner.py | 6 + application/api/answer/routes/base.py | 6 + application/guardrails/config.py | 58 ++++++- application/guardrails/engine.py | 2 +- application/guardrails/runtime.py | 4 +- application/guardrails/stream.py | 54 ++++++- application/guardrails/types.py | 9 ++ frontend/src/agents/NewAgent.tsx | 10 +- .../agents/components/GuardrailsSection.tsx | 8 +- .../src/conversation/conversationSlice.ts | 12 ++ .../conversation/sharedConversationSlice.ts | 22 +++ tests/guardrails/test_config_salvage.py | 103 +++++++++++++ tests/guardrails/test_redaction_boundaries.py | 143 ++++++++++++++++++ 13 files changed, 422 insertions(+), 15 deletions(-) create mode 100644 tests/guardrails/test_config_salvage.py create mode 100644 tests/guardrails/test_redaction_boundaries.py diff --git a/application/agents/headless_runner.py b/application/agents/headless_runner.py index 00c712b8..5d2da933 100644 --- a/application/agents/headless_runner.py +++ b/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 diff --git a/application/api/answer/routes/base.py b/application/api/answer/routes/base.py index 63aee214..60acef4c 100644 --- a/application/api/answer/routes/base.py +++ b/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 diff --git a/application/guardrails/config.py b/application/guardrails/config.py index e1fa115f..4bb54643 100644 --- a/application/guardrails/config.py +++ b/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): diff --git a/application/guardrails/engine.py b/application/guardrails/engine.py index 4d7bee15..cc882010 100644 --- a/application/guardrails/engine.py +++ b/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: diff --git a/application/guardrails/runtime.py b/application/guardrails/runtime.py index 8e1e9dac..ef18888f 100644 --- a/application/guardrails/runtime.py +++ b/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, } diff --git a/application/guardrails/stream.py b/application/guardrails/stream.py index 5a2fe98c..2d5f2eb3 100644 --- a/application/guardrails/stream.py +++ b/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( diff --git a/application/guardrails/types.py b/application/guardrails/types.py index 04330880..5018201f 100644 --- a/application/guardrails/types.py +++ b/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]: diff --git a/frontend/src/agents/NewAgent.tsx b/frontend/src/agents/NewAgent.tsx index ab40f21c..24ee5373 100644 --- a/frontend/src/agents/NewAgent.tsx +++ b/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 && (