Repository navigation
fix: fail closed for callable needs_approval on invalid/non-object JSON - #3870
baiyuxi930826 wants to merge 1 commit into
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dc5f5bf56a
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if callable(function_tool.needs_approval): | ||
| try: | ||
| parsed_args = json.loads(tool_call.arguments or "{}") | ||
| parsed = json.loads(tool_call.arguments or "{}") |
There was a problem hiding this comment.
When a provider emits arguments="", the or "{}" fallback converts that invalid JSON into an empty object, so a callable approval predicate can return False. Because _parse_function_tool_json_input() in src/agents/tool.py also treats an empty string as {}, a tool with no required parameters can then execute and produce side effects without approval. The same bypass exists in the Realtime path at src/agents/realtime/session.py:553; handle the empty string as fail-closed in both paths rather than substituting {}.
Useful? React with 👍 / 👎.
|
Resolved by #3867 |
Summary
Fixes #3863.
When
needs_approvalis a callable, invalid tool-call JSON was previously swallowed and replaced with{}before the predicate ran. Content-based gates (e.g."refund" in params.get("subject", "")) then returnedFalseand skipped HITL.This change fails closed:
JSONDecodeError→ require approval (True)dictJSON (null,[], scalars) → require approval (True)needs_approval=True→ unchangedSame fix in the realtime session path.
Test plan
pytest tests/test_function_needs_approval_invalid_json.py(5 passed)