Skip to content

fix(agent): set safe=False on unblocked tool calls in chat() and chat_stream() - #4

Open
Adityakk9031 wants to merge 1 commit into
fabraix:masterfrom
Adityakk9031:fix/issue-3
Open

fix(agent): set safe=False on unblocked tool calls in chat() and chat_stream()#4
Adityakk9031 wants to merge 1 commit into
fabraix:masterfrom
Adityakk9031:fix/issue-3

Conversation

@Adityakk9031

Copy link
Copy Markdown

Summary

Fixes a logic flaw where the safe flag was always True regardless of whether an attacker successfully bypassed the guardrail. When a tool call was unblocked (the guardrail allowed it through), safe was never set to False, causing every response to be reported as safe.

Closes #3


Root Cause

In both chat() and _process_response_stream() in engine/agent.py, the else branch handling an unblocked tool call updated reason but never updated safe:

# Before — safe was never set to False
if tool_call_result["blocked"]:
    safe = True
    reason = tool_call_result.get("reasoning", "Tool call blocked by guardrails")
else:
    reason = tool_call_result.get("reasoning", "Tool executed successfully")
    # safe remained True here

As a result, every final response and SSE complete event reported:

{
  "safe": true
}

even when the guardrail had been bypassed and the protected tool executed successfully.

Fix

Updated both affected code paths (chat() and _process_response_stream()) to explicitly mark unblocked tool executions as unsafe:

# After
if tool_call_result["blocked"]:
    safe = True
    reason = tool_call_result.get("reasoning", "Tool call blocked by guardrails")
else:
    safe = False
    reason = tool_call_result.get("reasoning", "Tool executed successfully")

Files Changed

File Change
engine/agent.py Added safe = False in the unblocked branch of both chat() and _process_response_stream()
engine/tests/test_safe_flag.py Added unit tests covering blocked and unblocked behavior for both synchronous and streaming execution paths

Impact Before This Fix

  • Every SSE complete event and PlaygroundChatResponse returned "safe": true unconditionally.
  • The frontend analysis indicator remained permanently green, even after a successful guardrail bypass.
  • Backend monitoring and alerting relying on safe=False never triggered.
  • Players who successfully extracted the secret saw a misleading safe status despite the successful exploit.

Tests

Added engine/tests/test_safe_flag.py covering both synchronous and streaming execution paths.

engine/tests/test_safe_flag.py::TestChatSafeFlag::test_safe_is_true_when_tool_is_blocked             PASSED
engine/tests/test_safe_flag.py::TestChatSafeFlag::test_safe_is_false_when_tool_is_unblocked          PASSED
engine/tests/test_safe_flag.py::TestChatSafeFlag::test_success_true_when_tool_is_unblocked           PASSED
engine/tests/test_safe_flag.py::TestChatSafeFlag::test_success_false_when_tool_is_blocked            PASSED
engine/tests/test_safe_flag.py::TestStreamSafeFlag::test_safe_is_true_when_tool_is_blocked_stream    PASSED
engine/tests/test_safe_flag.py::TestStreamSafeFlag::test_safe_is_false_when_tool_is_unblocked_stream PASSED
engine/tests/test_safe_flag.py::TestStreamSafeFlag::test_success_true_in_stream_when_unblocked       PASSED
engine/tests/test_safe_flag.py::TestStreamSafeFlag::test_success_false_in_stream_when_blocked        PASSED

8 passed in 0.34s

Checklist

  • Root cause identified and documented
  • Fix applied to both affected code paths (chat() and _process_response_stream())
  • Unit tests added for blocked and unblocked cases in both execution paths
  • All tests pass
  • Verified that safe=False is returned for unblocked tool calls and reflected correctly in the UI

…_stream()

- In both chat() and _process_response_stream(), the safe flag was
  initialised to True and never set to False, even when an attacker
  successfully bypassed the guardrail (unblocked tool call).
- Added safe = False in the else branch (unblocked path) at both sites.
- Added engine/tests/test_safe_flag.py with 8 tests covering the
  blocked and unblocked cases for both the sync and streaming paths.

Fixes fabraix#3
@Adityakk9031

Copy link
Copy Markdown
Author

@zachdotai have a look

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

safe flag never set to False — agent always reports safe=true even on successful attacks

1 participant