From b64fce59ea68af48614264a756003be8990969cb Mon Sep 17 00:00:00 2001 From: Alex Bujduveanu Date: Thu, 12 Nov 2026 19:17:26 +0000 Subject: [PATCH 1/4] fix(approval): evaluate require_approval predicates on schema-parsed 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> --- src/openrouter_agent/conversation_state.py | 37 ++- .../test_approval_predicate_parsed_args.py | 225 ++++++++++++++++++ 2 files changed, 257 insertions(+), 5 deletions(-) create mode 100644 tests/unit/test_approval_predicate_parsed_args.py diff --git a/src/openrouter_agent/conversation_state.py b/src/openrouter_agent/conversation_state.py index 7d1bf0e..2cce4d8 100644 --- a/src/openrouter_agent/conversation_state.py +++ b/src/openrouter_agent/conversation_state.py @@ -5,9 +5,11 @@ import time import uuid from dataclasses import replace -from typing import Any, Dict, List, Mapping, Optional, Sequence +from typing import Any, Dict, List, Mapping, Optional, Sequence, Tuple -from ._utils import dump, json_dumps, maybe_await +from pydantic import BaseModel + +from ._utils import dump, json_dumps, maybe_await, validate_schema from .tool_types import ( ConversationState, ParsedToolCall, @@ -189,14 +191,29 @@ def append_to_messages(current: Any, new_items: Sequence[Any]) -> List[Any]: return [*normalize_input_to_array(current), *list(new_items)] +def _parse_tool_arguments(tool: Tool, arguments: Any) -> Tuple[bool, Any]: + """Parse arguments with the schema `execute` validates through, so approval + predicates and `execute` see the same defaults and coercions.""" + if isinstance(arguments, str): + return False, arguments + try: + parsed = validate_schema(get_tool_function(tool).get("input_schema"), arguments) + except Exception: + return False, arguments + if isinstance(parsed, BaseModel): + parsed = parsed.model_dump() + return True, parsed + + async def tool_requires_approval( tool_call: ParsedToolCall, tools: Sequence[Tool], context: Mapping[str, Any], call_level_check: Any = None, ) -> bool: - if call_level_check is not None: - return bool(await maybe_await(call_level_check(tool_call, context))) + """Call-level checks get a copy of the call with parsed arguments when parsing + succeeds. Tool-level predicates get the parsed arguments; invalid or + non-mapping arguments fail closed (require approval).""" matching = next( ( candidate @@ -205,11 +222,21 @@ async def tool_requires_approval( ), None, ) + if call_level_check is not None: + if matching is None: + return bool(await maybe_await(call_level_check(tool_call, context))) + ok, parsed = _parse_tool_arguments(matching, tool_call.arguments) + if not ok: + return bool(await maybe_await(call_level_check(tool_call, context))) + return bool(await maybe_await(call_level_check(replace(tool_call, arguments=parsed), context))) if not matching: return False requirement = get_tool_function(matching).get("require_approval") if callable(requirement): - return bool(await maybe_await(requirement(tool_call.arguments, context))) + ok, parsed = _parse_tool_arguments(matching, tool_call.arguments) + if not ok or not isinstance(parsed, Mapping): + return True + return bool(await maybe_await(requirement(parsed, context))) return bool(requirement) diff --git a/tests/unit/test_approval_predicate_parsed_args.py b/tests/unit/test_approval_predicate_parsed_args.py new file mode 100644 index 0000000..5e71f2d --- /dev/null +++ b/tests/unit/test_approval_predicate_parsed_args.py @@ -0,0 +1,225 @@ +"""Approval predicates see the same schema-parsed arguments `execute` gets, +and schema-invalid arguments fail closed. Subset of upstream +`tests/unit/approval-gate-regressions.test.ts`. +""" + +from __future__ import annotations + +import dataclasses +from typing import Any, Dict, List, Tuple + +import pytest +from pydantic import BaseModel, field_validator + +from openrouter_agent import tool +from openrouter_agent.conversation_state import tool_requires_approval +from openrouter_agent.tool_executor import execute_regular_tool +from openrouter_agent.tool_types import ParsedToolCall, Tool + +CONTEXT: Dict[str, Any] = {"number_of_turns": 1} + + +class Recorder: + """Counts invocations and records arguments; returns a fixed value. + + A class (not `lambda: calls.append(...) or value`) so the return value is + explicit and mypy-clean -- see tests/unit/test_tool_execution_once.py. + """ + + def __init__(self, value: Any = None) -> None: + self.value = value + self.calls: List[Tuple[Any, ...]] = [] + + def __call__(self, *args: Any) -> Any: + self.calls.append(args) + return self.value + + @property + def count(self) -> int: + return len(self.calls) + + +def _dumped_first_args(recorder: Recorder) -> List[Any]: + return [call[0].model_dump() if isinstance(call[0], BaseModel) else call[0] for call in recorder.calls] + + +# --------------------------------------------------------------------------- +# Bug 1: the approval predicate must see the SAME arguments `execute` gets. +# --------------------------------------------------------------------------- + + +class DangerousDefaultArgs(BaseModel): + dangerous: bool = True + + +class AmountArgs(BaseModel): + amount: int + + +class DestructiveDefaultArgs(BaseModel): + destructive: bool = True + + +class TargetArgs(BaseModel): + target: str + + +class OkOutput(BaseModel): + ok: bool + + +async def test_applies_input_schema_defaults_before_function_require_approval() -> None: + predicate = Recorder() + + def require(params: Any, ctx: Any) -> bool: + predicate(params, ctx) + return params["dangerous"] is True + + conditional = tool( + name="conditional_action", + input_schema=DangerousDefaultArgs, + require_approval=require, + execute=lambda params, ctx: {}, + ) + + # The model emitted `{}`; the schema default makes `dangerous` True by the + # time execute runs, so the predicate must see it True too. + requires = await tool_requires_approval( + ParsedToolCall(id="1", name="conditional_action", arguments={}), [conditional], CONTEXT + ) + + # Pydantic models are model_dump()'d for the predicate (conversation_state.py + # _parse_tool_arguments): the predicate contract is a mapping, as upstream. + assert [call[0] for call in predicate.calls] == [{"dangerous": True}] + assert requires is True + + +async def test_applies_input_schema_coercions_before_function_require_approval() -> None: + predicate = Recorder() + + def require(params: Any, ctx: Any) -> bool: + predicate(params, ctx) + return bool(params["amount"] > 100) + + transfer = tool( + name="transfer", + input_schema=AmountArgs, + require_approval=require, + execute=lambda params, ctx: {}, + ) + + # String amount is coerced (Pydantic lax mode stands in for z.coerce), so + # the comparison is numeric, not lexicographic. + requires = await tool_requires_approval( + ParsedToolCall(id="2", name="transfer", arguments={"amount": "500"}), [transfer], CONTEXT + ) + + assert [call[0] for call in predicate.calls] == [{"amount": 500}] + assert requires is True + + +async def test_applies_schema_defaults_for_call_level_checks_without_mutating_call() -> None: + defaulted = tool( + name="call_level_defaulted_action", + input_schema=DestructiveDefaultArgs, + execute=lambda params, ctx: {}, + ) + tool_call = ParsedToolCall(id="call-level-default", name="call_level_defaulted_action", arguments={}) + call_level_check = Recorder(True) + + requires = await tool_requires_approval(tool_call, [defaulted], CONTEXT, call_level_check) + + assert call_level_check.calls == [(dataclasses.replace(tool_call, arguments={"destructive": True}), CONTEXT)] + assert tool_call.arguments == {} + assert requires is True + + +@pytest.mark.parametrize("decision", [False, True]) +async def test_honors_call_level_check_when_arguments_fail_schema_validation(decision: bool) -> None: + strict = tool(name="call_level_invalid_action", input_schema=TargetArgs, execute=lambda params, ctx: {}) + invalid_call = ParsedToolCall(id=f"call-level-invalid-{decision}", name="call_level_invalid_action", arguments={}) + call_level_check = Recorder(decision) + + assert await tool_requires_approval(invalid_call, [strict], CONTEXT, call_level_check) is decision + # Parsing failed, so the call-level check receives the raw call. + assert call_level_check.calls == [(invalid_call, CONTEXT)] + + +async def test_parses_original_wire_arguments_once_for_predicate_and_once_for_execution() -> None: + transform_calls = {"n": 0} + + class TransformedArgs(BaseModel): + value: str + + @field_validator("value") + @classmethod + def _suffix(cls, value: str) -> str: + transform_calls["n"] += 1 + return f"{value}:{transform_calls['n']}" + + predicate = Recorder(False) + execute = Recorder({}) + transformed = tool( + name="transformed_action", + input_schema=TransformedArgs, + require_approval=predicate, + execute=execute, + ) + tool_call = ParsedToolCall(id="non-idempotent-transform", name="transformed_action", arguments={"value": "wire"}) + + assert await tool_requires_approval(tool_call, [transformed], CONTEXT) is False + await execute_regular_tool(transformed, tool_call, CONTEXT) + + assert predicate.calls == [({"value": "wire:1"}, CONTEXT)] + assert _dumped_first_args(execute) == [{"value": "wire:2"}] + assert transform_calls["n"] == 2 + assert tool_call.arguments == {"value": "wire"} + + +async def test_fails_closed_when_arguments_fail_schema_validation() -> None: + predicate = Recorder(False) + strict = tool(name="strict_action", input_schema=TargetArgs, require_approval=predicate, execute=lambda p, c: {}) + + requires = await tool_requires_approval( + ParsedToolCall(id="3", name="strict_action", arguments={}), [strict], CONTEXT + ) + + assert requires is True + assert predicate.count == 0 + + +async def test_still_fails_closed_when_schema_parses_to_non_object_value() -> None: + predicate = Recorder(False) + weird = tool(name="weird_action", input_schema=TargetArgs, require_approval=predicate, execute=lambda p, c: {}) + # Mirrors upstream swapping in a schema whose parse succeeds with a + # non-record payload: a callable schema is honored by validate_schema. + weird["function"]["input_schema"] = lambda value: ["not", "a", "record"] + + requires = await tool_requires_approval( + ParsedToolCall(id="4", name="weird_action", arguments={"target": "x"}), [weird], CONTEXT + ) + + assert requires is True + assert predicate.count == 0 + + +async def test_still_fails_closed_on_schema_invalid_arguments_for_manual_tools() -> None: + predicate = Recorder(False) + # A manual tool: no execute / on_tool_called. The host runs it as-is, so a + # malformed call must not be waved past the gate. + manual_tool: Tool = { + "type": "function", + "function": {"name": "manual_action", "input_schema": TargetArgs, "require_approval": predicate}, + } + + requires = await tool_requires_approval( + ParsedToolCall(id="5", name="manual_action", arguments={}), [manual_tool], CONTEXT + ) + + assert requires is True + assert predicate.count == 0 + + +# --------------------------------------------------------------------------- +# Approval vs. PreToolUse-mutated arguments. +# --------------------------------------------------------------------------- From 18d0bd2ecb3b38af2c025208a9b1c787977673f8 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Thu, 12 Nov 2026 19:17:41 +0000 Subject: [PATCH 2/4] release: 0.8.0.post1 with the approval-argument fix 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. --- .github/workflows/publish.yaml | 7 +++++-- pyproject.toml | 2 +- upstreamer-changelog.md | 6 ++++++ uv.lock | 4 ++-- 4 files changed, 14 insertions(+), 5 deletions(-) diff --git a/.github/workflows/publish.yaml b/.github/workflows/publish.yaml index 2f8101d..1e034cb 100644 --- a/.github/workflows/publish.yaml +++ b/.github/workflows/publish.yaml @@ -108,9 +108,12 @@ jobs: https://github.com/OpenRouterTeam/typescript-agent.git /tmp/upstream-check git -C /tmp/upstream-check fetch -q --tags origin - TAG="@openrouter/agent@${VERSION}" + # A PEP 440 post-release (X.Y.Z.postN) is a downstream-only fix on top of + # the X.Y.Z port, so it is gated on the X.Y.Z release tag. + BASE_VERSION="${VERSION%%.post*}" + TAG="@openrouter/agent@${BASE_VERSION}" if ! git -C /tmp/upstream-check rev-parse -q --verify "refs/tags/$TAG" >/dev/null; then - echo "::error::Upstream has no release tag $TAG. This package's version tracks the ported @openrouter/agent version, so publishing $VERSION means upstream released it. Wait for the upstream release, or correct the version." + echo "::error::Upstream has no release tag $TAG. This package's version tracks the ported @openrouter/agent version (a .postN suffix marks a downstream-only fix), so publishing $VERSION means upstream released $BASE_VERSION. Wait for the upstream release, or correct the version." exit 1 fi diff --git a/pyproject.toml b/pyproject.toml index 9047731..f8d8c89 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -8,7 +8,7 @@ name = "openrouter-agent-sdk" # says plainly which upstream version it is a port of. The verifier enforces the # match against upstream's package.json during a sync. See the Package Version # section of .upstreamer/upstreamer.md. -version = "0.8.0" +version = "0.8.0.post1" description = "Python port of @openrouter/agent: OpenRouter tool orchestration, streaming, state, and format compatibility." readme = "README.md" requires-python = ">=3.10" diff --git a/upstreamer-changelog.md b/upstreamer-changelog.md index 29fa619..3ad267f 100644 --- a/upstreamer-changelog.md +++ b/upstreamer-changelog.md @@ -1,5 +1,11 @@ # openrouter-agent-sdk Changelog +## 0.8.0.post1 — approval fix + +Downstream-only fix on top of the 0.8.0 port; no new upstream work. + +- **Security:** callable `require_approval` predicates and call-level approval checks now see the arguments after `input_schema` parsing (coercions and defaults applied), the same values `execute` receives. Arguments that fail parsing require approval. Before this, a value such as the string `"true"` could pass a strict predicate and then run as boolean `True`. + ## 0.8.0 — first PyPI release First published release. The version is `0.8.0`, not `0.0.1`, because this diff --git a/uv.lock b/uv.lock index 09be5bd..da7fb1c 100644 --- a/uv.lock +++ b/uv.lock @@ -18,7 +18,7 @@ source = { registry = "https://pypi.org/simple" } dependencies = [ { name = "exceptiongroup", marker = "python_full_version < '3.11'" }, { name = "idna" }, - { name = "typing-extensions", marker = "python_full_version < '3.13'" }, + { name = "typing-extensions" }, ] sdist = { url = "https://files.pythonhosted.org/packages/3b/72/5562aabb8dd7181e8e860622a38bea08d17842b99ecd4c91f84ac95251b0/anyio-4.14.1.tar.gz", hash = "sha256:8d648a3544c1a700e3ff78615cd679e4c5c3f149904287e73687b2596963629e", size = 254831, upload-time = "2026-06-24T20:56:06.017Z" } wheels = [ @@ -379,7 +379,7 @@ wheels = [ [[package]] name = "openrouter-agent-sdk" -version = "0.8.0" +version = "0.8.0.post1" source = { editable = "." } dependencies = [ { name = "openrouter" }, From eed5637d76161eb8a881b0468228247f75599b8e Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Thu, 12 Nov 2026 19:56:24 +0000 Subject: [PATCH 3/4] release: 0.8.1 instead of 0.8.0.post1 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. --- .github/workflows/publish.yaml | 11 +++++++---- pyproject.toml | 2 +- upstreamer-changelog.md | 2 +- uv.lock | 2 +- 4 files changed, 10 insertions(+), 7 deletions(-) diff --git a/.github/workflows/publish.yaml b/.github/workflows/publish.yaml index 1e034cb..b7807c1 100644 --- a/.github/workflows/publish.yaml +++ b/.github/workflows/publish.yaml @@ -108,12 +108,15 @@ jobs: https://github.com/OpenRouterTeam/typescript-agent.git /tmp/upstream-check git -C /tmp/upstream-check fetch -q --tags origin - # A PEP 440 post-release (X.Y.Z.postN) is a downstream-only fix on top of - # the X.Y.Z port, so it is gated on the X.Y.Z release tag. - BASE_VERSION="${VERSION%%.post*}" + # Downstream-only patch releases: versions upstream never published, + # each mapped to the upstream release its tree is ported from. + case "$VERSION" in + 0.8.1) BASE_VERSION="0.8.0" ;; + *) BASE_VERSION="$VERSION" ;; + esac TAG="@openrouter/agent@${BASE_VERSION}" if ! git -C /tmp/upstream-check rev-parse -q --verify "refs/tags/$TAG" >/dev/null; then - echo "::error::Upstream has no release tag $TAG. This package's version tracks the ported @openrouter/agent version (a .postN suffix marks a downstream-only fix), so publishing $VERSION means upstream released $BASE_VERSION. Wait for the upstream release, or correct the version." + echo "::error::Upstream has no release tag $TAG. This package's version tracks the ported @openrouter/agent version so publishing $VERSION means upstream released $BASE_VERSION. Wait for the upstream release, or correct the version." exit 1 fi diff --git a/pyproject.toml b/pyproject.toml index f8d8c89..1bff328 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -8,7 +8,7 @@ name = "openrouter-agent-sdk" # says plainly which upstream version it is a port of. The verifier enforces the # match against upstream's package.json during a sync. See the Package Version # section of .upstreamer/upstreamer.md. -version = "0.8.0.post1" +version = "0.8.1" description = "Python port of @openrouter/agent: OpenRouter tool orchestration, streaming, state, and format compatibility." readme = "README.md" requires-python = ">=3.10" diff --git a/upstreamer-changelog.md b/upstreamer-changelog.md index 3ad267f..4f1ad77 100644 --- a/upstreamer-changelog.md +++ b/upstreamer-changelog.md @@ -1,6 +1,6 @@ # openrouter-agent-sdk Changelog -## 0.8.0.post1 — approval fix +## 0.8.1 — approval fix Downstream-only fix on top of the 0.8.0 port; no new upstream work. diff --git a/uv.lock b/uv.lock index da7fb1c..0283852 100644 --- a/uv.lock +++ b/uv.lock @@ -379,7 +379,7 @@ wheels = [ [[package]] name = "openrouter-agent-sdk" -version = "0.8.0.post1" +version = "0.8.1" source = { editable = "." } dependencies = [ { name = "openrouter" }, From 0b0aa7ca9274acba33967692dafdf5ed3e5348bb Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Thu, 12 Nov 2026 19:58:48 +0000 Subject: [PATCH 4/4] ci(publish): restore comma in the tag-check error --- .github/workflows/publish.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/publish.yaml b/.github/workflows/publish.yaml index b7807c1..934bd53 100644 --- a/.github/workflows/publish.yaml +++ b/.github/workflows/publish.yaml @@ -116,7 +116,7 @@ jobs: esac TAG="@openrouter/agent@${BASE_VERSION}" if ! git -C /tmp/upstream-check rev-parse -q --verify "refs/tags/$TAG" >/dev/null; then - echo "::error::Upstream has no release tag $TAG. This package's version tracks the ported @openrouter/agent version so publishing $VERSION means upstream released $BASE_VERSION. Wait for the upstream release, or correct the version." + echo "::error::Upstream has no release tag $TAG. This package's version tracks the ported @openrouter/agent version, so publishing $VERSION means upstream released $BASE_VERSION. Wait for the upstream release, or correct the version." exit 1 fi