From e5d361876a90b7a982bfec686fca6eb19571ffed Mon Sep 17 00:00:00 2001 From: Miracle778 <37257176+Miracle778@users.noreply.github.com> Date: Fri, 3 Jul 2026 16:08:41 +0800 Subject: [PATCH] feat(guardrails): persist security interventions as run events (#3837) * feat: record guardrail decisions in run events Persist security-relevant GuardrailMiddleware outcomes (deny and provider-error fail-open/fail-closed) as middleware:guardrail run events via RunJournal.record_middleware(), mirroring SafetyFinishReasonMiddleware. Recording is best-effort: it reads __run_journal from the runtime context and swallows persistence failures, so tool execution behavior is unchanged. The audit payload records tool name/id, agent id, subagent flag, user role, allow decision, policy id, reason codes/messages, fail_closed mode, and provider_error flag. Tool input/args and identity fields (user_id, oauth_*) are deliberately excluded to avoid persisting the sensitive content being blocked. The fail-closed provider-error branch returns the denied message directly from the except block so it records exactly once and does not fall through to the generic deny branch. * docs: clarify guardrail journal runtime boundary * fix: preserve subagent attribution in guardrail events * test: align guardrail event attribution fixtures * refactor(guardrails): resolve runtime context once per tool call Extract `_resolve_context` and thread the already-resolved context dict through `_build_request` and `_record_guardrail_event` so the getattr/runtime.context chain runs once per wrap_tool_call instead of twice. Also document the `is_subagent` field boundary: native subagents do not inherit __run_journal, so the field is structurally False in persisted records today; custom runtimes may still supply it with attribution. Addresses review feedback on #3837 (context-read-twice cleanup and the is_subagent trade-off note). No behavior change. --- .../harness/deerflow/guardrails/middleware.py | 110 +++++++++- backend/tests/test_guardrail_middleware.py | 189 +++++++++++++++++- 2 files changed, 294 insertions(+), 5 deletions(-) diff --git a/backend/packages/harness/deerflow/guardrails/middleware.py b/backend/packages/harness/deerflow/guardrails/middleware.py index c76d87265..ed0df4183 100644 --- a/backend/packages/harness/deerflow/guardrails/middleware.py +++ b/backend/packages/harness/deerflow/guardrails/middleware.py @@ -16,6 +16,8 @@ from deerflow.guardrails.provider import GuardrailDecision, GuardrailProvider, G logger = logging.getLogger(__name__) +_REASON_MESSAGE_LIMIT = 500 + class GuardrailMiddleware(AgentMiddleware[AgentState]): """Evaluate tool calls against a GuardrailProvider before execution. @@ -31,11 +33,13 @@ class GuardrailMiddleware(AgentMiddleware[AgentState]): self.fail_closed = fail_closed self.passport = passport - def _build_request(self, request: ToolCallRequest) -> GuardrailRequest: + @staticmethod + def _resolve_context(request: ToolCallRequest) -> dict: runtime = getattr(request, "runtime", None) context = getattr(runtime, "context", None) if runtime is not None else None - context = context if isinstance(context, dict) else {} + return context if isinstance(context, dict) else {} + def _build_request(self, request: ToolCallRequest, context: dict) -> GuardrailRequest: return GuardrailRequest( tool_name=str(request.tool_call.get("name", "")), tool_input=request.tool_call.get("args", {}), @@ -63,13 +67,64 @@ class GuardrailMiddleware(AgentMiddleware[AgentState]): status="error", ) + def _record_guardrail_event( + self, + context: dict, + guardrail_request: GuardrailRequest, + decision: GuardrailDecision, + *, + action: str, + provider_error: bool, + ) -> None: + """Persist a security-relevant guardrail decision to RunJournal. + + This follows the optional-Journal pattern used by existing middleware: + audit persistence is best-effort and must never change tool execution + behavior. Runtimes without ``__run_journal`` (including embedded and + subagent execution) skip persistence. + """ + journal = context.get("__run_journal") + if journal is None: + return + + reason_codes = [reason.code for reason in decision.reasons if reason.code] + reason_messages = [reason.message[:_REASON_MESSAGE_LIMIT] for reason in decision.reasons if reason.message] + + changes = { + "tool_name": guardrail_request.tool_name, + "tool_call_id": guardrail_request.tool_call_id, + "agent_id": guardrail_request.agent_id, + # Native subagents do not currently inherit __run_journal; custom + # runtimes may still provide one with subagent attribution. + "is_subagent": guardrail_request.is_subagent, + "user_role": guardrail_request.user_role, + "allow": decision.allow, + "policy_id": decision.policy_id, + "reason_codes": reason_codes, + "reason_messages": reason_messages, + "fail_closed": self.fail_closed, + "provider_error": provider_error, + } + + try: + journal.record_middleware( + tag="guardrail", + name=type(self).__name__, + hook="wrap_tool_call", + action=action, + changes=changes, + ) + except Exception: # noqa: BLE001 + logger.debug("Failed to record middleware:guardrail event", exc_info=True) + @override def wrap_tool_call( self, request: ToolCallRequest, handler: Callable[[ToolCallRequest], ToolMessage | Command], ) -> ToolMessage | Command: - gr = self._build_request(request) + context = self._resolve_context(request) + gr = self._build_request(request, context) try: decision = self.provider.evaluate(gr) except GraphBubbleUp: @@ -79,10 +134,33 @@ class GuardrailMiddleware(AgentMiddleware[AgentState]): logger.exception("Guardrail provider error (sync)") if self.fail_closed: decision = GuardrailDecision(allow=False, reasons=[GuardrailReason(code="oap.evaluator_error", message="guardrail provider error (fail-closed)")]) + self._record_guardrail_event( + context, + gr, + decision, + action="deny_tool_call", + provider_error=True, + ) + return self._build_denied_message(request, decision) else: + decision = GuardrailDecision(allow=True, reasons=[GuardrailReason(code="oap.evaluator_error", message="guardrail provider error (fail-open)")]) + self._record_guardrail_event( + context, + gr, + decision, + action="allow_tool_call_after_provider_error", + provider_error=True, + ) return handler(request) if not decision.allow: logger.warning("Guardrail denied: tool=%s policy=%s code=%s", gr.tool_name, decision.policy_id, decision.reasons[0].code if decision.reasons else "unknown") + self._record_guardrail_event( + context, + gr, + decision, + action="deny_tool_call", + provider_error=False, + ) return self._build_denied_message(request, decision) return handler(request) @@ -92,7 +170,8 @@ class GuardrailMiddleware(AgentMiddleware[AgentState]): request: ToolCallRequest, handler: Callable[[ToolCallRequest], Awaitable[ToolMessage | Command]], ) -> ToolMessage | Command: - gr = self._build_request(request) + context = self._resolve_context(request) + gr = self._build_request(request, context) try: decision = await self.provider.aevaluate(gr) except GraphBubbleUp: @@ -102,9 +181,32 @@ class GuardrailMiddleware(AgentMiddleware[AgentState]): logger.exception("Guardrail provider error (async)") if self.fail_closed: decision = GuardrailDecision(allow=False, reasons=[GuardrailReason(code="oap.evaluator_error", message="guardrail provider error (fail-closed)")]) + self._record_guardrail_event( + context, + gr, + decision, + action="deny_tool_call", + provider_error=True, + ) + return self._build_denied_message(request, decision) else: + decision = GuardrailDecision(allow=True, reasons=[GuardrailReason(code="oap.evaluator_error", message="guardrail provider error (fail-open)")]) + self._record_guardrail_event( + context, + gr, + decision, + action="allow_tool_call_after_provider_error", + provider_error=True, + ) return await handler(request) if not decision.allow: logger.warning("Guardrail denied: tool=%s policy=%s code=%s", gr.tool_name, decision.policy_id, decision.reasons[0].code if decision.reasons else "unknown") + self._record_guardrail_event( + context, + gr, + decision, + action="deny_tool_call", + provider_error=False, + ) return self._build_denied_message(request, decision) return await handler(request) diff --git a/backend/tests/test_guardrail_middleware.py b/backend/tests/test_guardrail_middleware.py index 5e1efe2fb..8985d1a97 100644 --- a/backend/tests/test_guardrail_middleware.py +++ b/backend/tests/test_guardrail_middleware.py @@ -15,10 +15,33 @@ from deerflow.guardrails.provider import GuardrailDecision, GuardrailReason, Gua # --- Helpers --- -def _make_tool_call_request(name: str = "bash", args: dict | None = None, call_id: str = "call_1"): +class _FakeRuntime: + def __init__(self, context: dict | None = None): + self.context = context or {} + + +class _FakeJournal: + def __init__(self, *, fail: bool = False): + self.fail = fail + self.calls: list[dict] = [] + + def record_middleware(self, **kwargs): + if self.fail: + raise RuntimeError("journal unavailable") + self.calls.append(kwargs) + + +def _make_tool_call_request( + name: str = "bash", + args: dict | None = None, + call_id: str = "call_1", + *, + context: dict | None = None, +): """Create a mock ToolCallRequest.""" req = MagicMock() req.tool_call = {"name": name, "args": args or {}, "id": call_id} + req.runtime = _FakeRuntime(context) return req @@ -299,6 +322,170 @@ class TestGuardrailMiddleware: with pytest.raises(GraphBubbleUp): asyncio.run(run()) + # Journal: a denied tool call records the complete guardrail audit event. + def test_denied_tool_records_guardrail_event(self): + journal = _FakeJournal() + mw = GuardrailMiddleware(_DenyAllProvider(), passport="agent_id") + req = _make_tool_call_request( + "bash", + args={"command": "cat secret.txt"}, + call_id="tool_call_1", + context={ + "__run_journal": journal, + "user_role": "user", + }, + ) + result = mw.wrap_tool_call(req, MagicMock()) + + assert result.status == "error" + assert len(journal.calls) == 1 + event = journal.calls[0] + assert event["tag"] == "guardrail" + assert event["name"] == "GuardrailMiddleware" + assert event["hook"] == "wrap_tool_call" + assert event["action"] == "deny_tool_call" + changes = event["changes"] + assert changes["tool_name"] == "bash" + assert changes["tool_call_id"] == "tool_call_1" + assert changes["agent_id"] == "agent_id" + assert changes["is_subagent"] is False + assert changes["user_role"] == "user" + assert changes["allow"] is False + assert changes["policy_id"] == "test.deny.v1" + assert changes["reason_codes"] == ["oap.denied"] + assert changes["reason_messages"] == ["all tools blocked"] + assert changes["fail_closed"] is True + assert changes["provider_error"] is False + assert "tool_input" not in changes + assert "args" not in changes + assert "command" not in changes + assert "user_id" not in changes + assert "oauth_provider" not in changes + assert "oauth_id" not in changes + + # Journal: a fail-closed provider error is recorded as a denied tool call. + def test_fail_closed_provider_error_records_guardrail_event(self): + journal = _FakeJournal() + mw = GuardrailMiddleware(_ExplodingProvider(), fail_closed=True) + req = _make_tool_call_request("bash", context={"__run_journal": journal}) + handler = MagicMock() + + result = mw.wrap_tool_call(req, handler) + + handler.assert_not_called() + assert result.status == "error" + assert len(journal.calls) == 1 + event = journal.calls[0] + assert event["action"] == "deny_tool_call" + changes = event["changes"] + assert changes["allow"] is False + assert changes["reason_codes"] == ["oap.evaluator_error"] + assert changes["provider_error"] is True + assert changes["fail_closed"] is True + + # Journal: a fail-open provider error is recorded without blocking the tool. + def test_fail_open_provider_error_records_guardrail_event_and_allows_handler(self): + journal = _FakeJournal() + mw = GuardrailMiddleware(_ExplodingProvider(), fail_closed=False) + req = _make_tool_call_request("bash", context={"__run_journal": journal}) + expected = MagicMock() + handler = MagicMock(return_value=expected) + + result = mw.wrap_tool_call(req, handler) + + handler.assert_called_once_with(req) + assert result is expected + assert len(journal.calls) == 1 + event = journal.calls[0] + assert event["action"] == "allow_tool_call_after_provider_error" + changes = event["changes"] + assert changes["allow"] is True + assert changes["reason_codes"] == ["oap.evaluator_error"] + assert changes["provider_error"] is True + assert changes["fail_closed"] is False + + # Journal: ordinary allowed decisions do not create guardrail audit events. + def test_allowed_tool_does_not_record_guardrail_event(self): + journal = _FakeJournal() + mw = GuardrailMiddleware(_AllowAllProvider()) + req = _make_tool_call_request("web_search", context={"__run_journal": journal}) + expected = MagicMock() + handler = MagicMock(return_value=expected) + + result = mw.wrap_tool_call(req, handler) + + assert result is expected + assert journal.calls == [] + + # Journal: a recording failure must not alter the guardrail denial outcome. + def test_guardrail_event_recording_failure_does_not_change_denial(self): + journal = _FakeJournal(fail=True) + mw = GuardrailMiddleware(_DenyAllProvider()) + req = _make_tool_call_request("bash", context={"__run_journal": journal}) + handler = MagicMock() + + result = mw.wrap_tool_call(req, handler) + + handler.assert_not_called() + assert result.status == "error" + assert "oap.denied" in result.content + + # Journal: the async denial path records the same guardrail audit event. + def test_async_denied_tool_records_guardrail_event(self): + journal = _FakeJournal() + mw = GuardrailMiddleware(_DenyAllProvider(), passport="agent_id") + req = _make_tool_call_request( + "bash", + call_id="async_call_1", + context={"__run_journal": journal}, + ) + + async def handler(r): + return MagicMock() + + async def run(): + return await mw.awrap_tool_call(req, handler) + + result = asyncio.run(run()) + + assert result.status == "error" + assert len(journal.calls) == 1 + event = journal.calls[0] + assert event["tag"] == "guardrail" + assert event["hook"] == "wrap_tool_call" + assert event["action"] == "deny_tool_call" + changes = event["changes"] + assert changes["tool_name"] == "bash" + assert changes["tool_call_id"] == "async_call_1" + assert changes["agent_id"] == "agent_id" + assert changes["is_subagent"] is False + assert changes["allow"] is False + assert changes["provider_error"] is False + + # Journal: the async fail-open path records the error and still runs the tool. + def test_async_fail_open_provider_error_records_guardrail_event_and_allows_handler(self): + journal = _FakeJournal() + mw = GuardrailMiddleware(_ExplodingProvider(), fail_closed=False) + req = _make_tool_call_request("bash", context={"__run_journal": journal}) + expected = MagicMock() + + async def handler(r): + return expected + + async def run(): + return await mw.awrap_tool_call(req, handler) + + result = asyncio.run(run()) + + assert result is expected + assert len(journal.calls) == 1 + event = journal.calls[0] + assert event["action"] == "allow_tool_call_after_provider_error" + changes = event["changes"] + assert changes["allow"] is True + assert changes["provider_error"] is True + assert changes["fail_closed"] is False + class TestGuardrailRequestAttribution: """Tests for GuardrailRequest runtime attribution fields."""