Repository navigation
fix(approval): evaluate require_approval predicates on schema-parsed arguments - #31
Conversation
…arguments Callable require_approval predicates and call-level approval checks received the raw model arguments, while execute received the arguments after input_schema validation. Coercions and defaults could therefore make a strict predicate allow a call whose executed input needed approval. Parse arguments with the tool's input_schema before the approval check and pass the parsed mapping to tool-level predicates. Call-level checks get a copy of the call with parsed arguments. Arguments that fail parsing, or parse to a non-mapping value, require approval. Ported from the upstream approval-gate regression suite. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Bump to the PEP 440 post-release 0.8.0.post1: the tree is still the 0.8.0 port plus a downstream-only fix, so the publish guard now resolves X.Y.Z.postN to the upstream X.Y.Z release tag.
|
I'll fix CI failures and address comments from users with write access that start with 'Devin'.
|
Use an integer patch version for the downstream-only fix. The publish guard maps 0.8.1 explicitly to the @openrouter/agent@0.8.0 tag it is ported from, since upstream never released 0.8.1.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
There was a problem hiding this comment.
Verdict: no blockers. Posting as COMMENT, not APPROVE, because the code was written by the same automation that is reviewing it; a human reviewer should approve.
Estimated impact: medium. Changes which calls need human approval and the release gate; failure modes fail closed (extra approval prompts) rather than open.
Risk: implementation low, premise low, impact medium.
Checked: _parse_tool_arguments uses the same validate_schema(input_schema, arguments) call as execute_regular_tool, so predicates see the values execute receives (model_dump() of the same model). The only approval entry point is tool_requires_approval via partition_tool_calls; _classify already routes raw-string arguments to approval. The publish case only maps 0.8.1; every other version still needs its own upstream tag, and the ahead-of-tag check still runs against @openrouter/agent@0.8.0.
Summary
Callable
require_approvalpredicates and call-level approval checks received the raw model arguments, whileexecutereceived the arguments afterinput_schemavalidation. A coerced or defaulted value could pass a strict predicate and then run as the dangerous value without approval (for example the string"true"failingargs["force"] is Trueand then executing asTrue).tool_requires_approvalnow parses arguments with the tool'sinput_schemafirst (_parse_tool_arguments). Tool-level predicates get the parsed mapping; arguments that fail parsing or don't produce a mapping require approval. Call-level checks get a copy of the call with parsed arguments. This is the approval part oftool_requires_approvalfrom port: sync with @openrouter/agent upstream (EVAL FAILED — do not merge) #29.tests/unit/test_approval_predicate_parsed_args.py(defaults, coercion, call-level checks, invalid and non-object arguments).0.8.1, a downstream-only patch on the 0.8.0 port. Upstream never published0.8.1, so the publish guard maps it explicitly to the@openrouter/agent@0.8.0tag for its tag and ahead-of-tag checks. Changelog entry added.Not included: re-checking approval after a
PreToolUsehook rewrites arguments (depends on the largermodel_result.pyrewrite in #29).Tests:
pytest tests/unit(132 passed),ruff check,ruff format --check,mypy src tests,uv lock --check.