-
Notifications
You must be signed in to change notification settings - Fork 2.9k
Fix: Input guardrails now properly block tool execution #1914
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
This fixes issue openai#889 where input guardrails were running in parallel with agent execution, allowing tools to run even when guardrails should block them. Changes: - Modified non-streaming path to run guardrails sequentially before _run_single_turn - Modified streaming path to await guardrails completion before starting the streamed agent turn - Added immediate exception raising in _run_input_guardrails_with_queue when tripwire is triggered This ensures that when a guardrail triggers, no LLM calls or tool executions happen, preventing unnecessary token consumption and potential security issues.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR.
| if current_turn == 1: | ||
| # Run the input guardrails in the background and put the results on the queue | ||
| streamed_result._input_guardrails_task = asyncio.create_task( | ||
| cls._run_input_guardrails_with_queue( | ||
| starting_agent, | ||
| starting_agent.input_guardrails + (run_config.input_guardrails or []), | ||
| ItemHelpers.input_to_new_input_list(prepared_input), | ||
| context_wrapper, | ||
| streamed_result, | ||
| current_span, | ||
| ) | ||
| # Run input guardrails first, before starting the streamed agent turn. | ||
| # This prevents tools from executing when guardrails should block the input. | ||
| # We await this to ensure guardrails complete before the agent starts. | ||
| await cls._run_input_guardrails_with_queue( | ||
| starting_agent, | ||
| starting_agent.input_guardrails + (run_config.input_guardrails or []), | ||
| ItemHelpers.input_to_new_input_list(prepared_input), | ||
| context_wrapper, | ||
| streamed_result, | ||
| current_span, | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Guardrail tripwire causes stream_events hang
Awaiting _run_input_guardrails_with_queue means a tripwire now raises before anything is queued on the streaming event queue. If a caller starts iterating RunResultStreaming.stream_events() while guardrails are still running, the iterator runs _check_errors() once (no error yet) and then awaits self._event_queue.get(). When the guardrail subsequently raises, _start_streaming exits without ever enqueuing a QueueCompleteSentinel or any event, so the consumer remains blocked forever and never receives the InputGuardrailTripwireTriggered exception even though _run_impl_task has failed. Consider notifying the event queue or setting is_complete before raising so that the stream can terminate instead of deadlocking.
Useful? React with 👍 / 👎.
|
Thanks for sending this PR. However, getting rid of parallel tool execution by removing |
Summary
Fixes #889 - Input guardrails were running in parallel with agent execution, allowing tools to execute even when guardrails should block them. This caused unnecessary token consumption and potential security issues.
Changes
Non-streaming path (Runner.run)
_run_single_turnStreaming path (Runner.run_streamed)
_run_input_guardrails_with_queuefrom background task to awaited call_run_input_guardrails_with_queueImpact
Before this fix:
asyncio.gather)After this fix:
Testing
Reviewers
This addresses the concern raised by multiple users in #889 where tools were executing despite guardrail tripwires being triggered. The fix ensures guardrails work as documented: blocking agent execution before any resources are consumed.