diff --git a/implementations/acr-control-plane/docs/sdk.md b/implementations/acr-control-plane/docs/sdk.md index 8478237..bb3fc8e 100644 --- a/implementations/acr-control-plane/docs/sdk.md +++ b/implementations/acr-control-plane/docs/sdk.md @@ -56,6 +56,14 @@ result = session.evaluate_action( The Python SDK also includes a real adapter layer for LangGraph/LangChain-style tools. +**Optional dependency limitation (2026-10-25):** the `integrations` extra and the +legacy LangGraph example dependency family are not audit-clean. Do not install +those optional dependencies for production evaluation pending a separately reviewed +migration. The SDK/base-package security fix does not remediate them; the base +`pip install acr-control-plane` path does not install them. See the +[example installation warning](../examples/langgraph_guard_example.md#install). +The plain `guard_tool` and `guard_async_tool` wrappers do not require LangChain. + Use: - `guard_tool(...)` @@ -92,6 +100,14 @@ That wrapper will: - apply modified parameters on `modify` - return gateway-managed execution output when configured not to execute locally +The Python SDK rejects ALLOW/MODIFY on non-200 evaluation responses. A MODIFY +response must include an explicit replacement action and parameters for the +requested tool; missing transformations and changed tool names raise an SDK +error rather than executing the original call. An explicitly empty parameter +object remains valid and follows the wrapped function's normal default-argument +behavior. These checks do not authenticate a compromised gateway or provide +approval replay/concurrency protection. + ## TypeScript SDK The TypeScript package lives at: diff --git a/implementations/acr-control-plane/examples/langgraph_guard_example.md b/implementations/acr-control-plane/examples/langgraph_guard_example.md index c53bb35..40b513f 100644 --- a/implementations/acr-control-plane/examples/langgraph_guard_example.md +++ b/implementations/acr-control-plane/examples/langgraph_guard_example.md @@ -7,6 +7,14 @@ search or real email is sent. ## Install +> **Security limitation (2026-10-25):** the optional legacy LangChain/LangGraph +> dependency family below is **not audit-clean**. Do not install it for production +> evaluation pending a separately reviewed dependency migration. The SDK/base-package +> security fix does not remediate these optional dependencies. The base +> `pip install acr-control-plane` path does not install them. This command is +> retained only for isolated, disposable regression reproduction without secrets +> or untrusted inputs. + From `implementations/acr-control-plane/`, in your activated development virtual environment: @@ -14,7 +22,7 @@ environment: python -m pip install -e '.[dev,integrations]' 'langgraph>=0.3,<0.7' ``` -This example uses the repository's supported LangChain Core 0.3 integration range. +This example uses the repository's legacy LangChain Core 0.3 integration range. The LangGraph bound keeps installation within the matching pre-1.0 framework family; it is not a claim of compatibility with every newer LangGraph release. diff --git a/implementations/acr-control-plane/pyproject.toml b/implementations/acr-control-plane/pyproject.toml index 7c2998f..0f4fa89 100644 --- a/implementations/acr-control-plane/pyproject.toml +++ b/implementations/acr-control-plane/pyproject.toml @@ -37,15 +37,15 @@ dependencies = [ "passlib[bcrypt]>=1.7.4,<2.0.0", # OpenTelemetry — core (stable releases) - "opentelemetry-api>=1.29.0,<2.0.0", - "opentelemetry-sdk>=1.29.0,<2.0.0", - "opentelemetry-exporter-otlp-proto-http>=1.29.0,<2.0.0", + "opentelemetry-api>=1.45.1,<2.0.0", + "opentelemetry-sdk>=1.45.1,<2.0.0", + "opentelemetry-exporter-otlp-proto-http>=1.45.1,<2.0.0", # OpenTelemetry — instrumentation packages track the core version family. - # Pinned to <0.60b0 to prevent unreviewed major jumps. - "opentelemetry-instrumentation-fastapi==0.59b0", - "opentelemetry-instrumentation-httpx==0.59b0", - "opentelemetry-instrumentation-sqlalchemy==0.59b0", + # This reviewed family permits the CVE-2026-114603 fix in API 1.45.1. + "opentelemetry-instrumentation-fastapi==0.66b1", + "opentelemetry-instrumentation-httpx==0.66b1", + "opentelemetry-instrumentation-sqlalchemy==0.66b1", # Circuit breakers "aiobreaker>=1.1", diff --git a/implementations/acr-control-plane/src/acr/sdk/client.py b/implementations/acr-control-plane/src/acr/sdk/client.py index 562db7c..e805f88 100644 --- a/implementations/acr-control-plane/src/acr/sdk/client.py +++ b/implementations/acr-control-plane/src/acr/sdk/client.py @@ -46,7 +46,9 @@ def _parse_model(response: httpx.Response, model_type): return model_type.model_validate(response.json()) -def _parse_evaluate_response(response: httpx.Response) -> EvaluateResponse: +def _parse_evaluate_response( + response: httpx.Response, *, expected_tool_name: str +) -> EvaluateResponse: payload = _parse_json_response(response) if response.status_code not in _EVALUATE_STATUS_CODES: raise ACRHTTPError( @@ -60,7 +62,34 @@ def _parse_evaluate_response(response: httpx.Response) -> EvaluateResponse: message="Evaluate response body was not JSON", body=payload, ) - return EvaluateResponse.model_validate(payload) + result = EvaluateResponse.model_validate(payload) + # Only a successful evaluation may authorize execution. Preserve structured + # deny/escalate results on supported failure statuses, but never let a + # contradictory response body turn an error or pending status into allow. + if response.status_code != 200 and result.is_allowed: + raise ACRHTTPError( + status_code=response.status_code, + message="Non-success evaluate response cannot authorize execution", + body=payload, + ) + # Validate transformation completeness and action identity for raw SDK + # callers too, not only callers using the optional local guard wrappers. + modified = result.modified_action + if result.decision == "modify" and modified is None: + raise ACRHTTPError( + status_code=response.status_code, + message="MODIFY response requires an explicit modified action", + body=payload, + ) + if modified is not None and ( + modified.tool_name != expected_tool_name or "parameters" not in modified.model_fields_set + ): + raise ACRHTTPError( + status_code=response.status_code, + message="Modified action must match the requested tool and include explicit parameters", + body=payload, + ) + return result class ACRClient: @@ -171,7 +200,7 @@ def evaluate(self, request: EvaluateRequest, *, access_token: str) -> EvaluateRe headers=self._agent_headers(access_token), json=_jsonable(request), ) - return _parse_evaluate_response(response) + return _parse_evaluate_response(response, expected_tool_name=request.action.tool_name) def evaluate_action( self, @@ -307,7 +336,7 @@ async def evaluate(self, request: EvaluateRequest, *, access_token: str) -> Eval headers=self._agent_headers(access_token), json=_jsonable(request), ) - return _parse_evaluate_response(response) + return _parse_evaluate_response(response, expected_tool_name=request.action.tool_name) async def evaluate_action( self, diff --git a/implementations/acr-control-plane/src/acr/sdk/langgraph.py b/implementations/acr-control-plane/src/acr/sdk/langgraph.py index e04c49c..6e18169 100644 --- a/implementations/acr-control-plane/src/acr/sdk/langgraph.py +++ b/implementations/acr-control-plane/src/acr/sdk/langgraph.py @@ -9,7 +9,7 @@ from acr.gateway.models import EvaluateResponse, IntentRequest from acr.sdk.client import ACRAgentSession, AsyncACRAgentSession -from acr.sdk.errors import ACRDeniedError, ACREscalatedError +from acr.sdk.errors import ACRDeniedError, ACREscalatedError, ACRSDKError def _bind_arguments(func: Callable[..., Any], *args: Any, **kwargs: Any) -> dict[str, Any]: @@ -27,9 +27,16 @@ def _coerce_intent(intent: IntentRequest | dict[str, Any] | None) -> IntentReque def _resolve_effective_parameters( original_parameters: dict[str, Any], response: EvaluateResponse, + tool_name: str, ) -> dict[str, Any]: if response.modified_action is None: + if response.decision == "modify": + raise ACRSDKError("MODIFY response requires an explicit modified action") return original_parameters + if response.modified_action.tool_name != tool_name: + raise ACRSDKError("Modified action does not match the guarded tool") + if "parameters" not in response.modified_action.model_fields_set: + raise ACRSDKError("Modified action requires explicit parameters") return dict(response.modified_action.parameters) @@ -72,9 +79,11 @@ def wrapped(*args: Any, **kwargs: Any) -> Any: intent=intent, ) _handle_blocking_decision(response) + effective_parameters = _resolve_effective_parameters( + parameters, response, resolved_tool_name + ) if response.execution_result is not None and not execute_locally_on_allow: return response.execution_result - effective_parameters = _resolve_effective_parameters(parameters, response) return func(**effective_parameters) return wrapped @@ -107,9 +116,11 @@ async def wrapped(*args: Any, **kwargs: Any) -> Any: intent=intent, ) _handle_blocking_decision(response) + effective_parameters = _resolve_effective_parameters( + parameters, response, resolved_tool_name + ) if response.execution_result is not None and not execute_locally_on_allow: return response.execution_result - effective_parameters = _resolve_effective_parameters(parameters, response) result = func(**effective_parameters) if inspect.isawaitable(result): return await result diff --git a/implementations/acr-control-plane/tests/test_otel_dependency_compatibility.py b/implementations/acr-control-plane/tests/test_otel_dependency_compatibility.py new file mode 100644 index 0000000..caa9aff --- /dev/null +++ b/implementations/acr-control-plane/tests/test_otel_dependency_compatibility.py @@ -0,0 +1,35 @@ +"""Bounded, offline regressions for the patched W3C trace-context parser.""" + +from opentelemetry.trace import get_current_span +from opentelemetry.trace.propagation.tracecontext import TraceContextTextMapPropagator + + +def test_trace_context_preserves_valid_trace_and_state(): + context = TraceContextTextMapPropagator().extract( + { + "traceparent": "00-0af7651916cd43dd8448eb211c80319c-b7ad6b7169203331-01", + "tracestate": "vendor=value,other=second", + } + ) + span_context = get_current_span(context).get_span_context() + assert span_context.is_valid + assert span_context.trace_id == int("0af7651916cd43dd8448eb211c80319c", 16) + assert list(span_context.trace_state.items()) == [("vendor", "value"), ("other", "second")] + + +def test_trace_state_rejects_combined_headers_over_security_limit(): + # Each member is individually valid, and the 32-member limit is respected. + # The combined header is just over the patched 8192-character limit. This + # tests bounded rejection without a timing assertion or a large CPU payload. + members = [f"k{index:02d}=" + "x" * 253 for index in range(32)] + header = ",".join(members) + assert len(header) == 8255 + context = TraceContextTextMapPropagator().extract( + { + "traceparent": "00-0af7651916cd43dd8448eb211c80319c-b7ad6b7169203331-01", + "tracestate": header, + } + ) + span_context = get_current_span(context).get_span_context() + assert span_context.is_valid + assert list(span_context.trace_state.items()) == [] diff --git a/implementations/acr-control-plane/tests/test_sdk_fail_closed_responses.py b/implementations/acr-control-plane/tests/test_sdk_fail_closed_responses.py new file mode 100644 index 0000000..82a02d5 --- /dev/null +++ b/implementations/acr-control-plane/tests/test_sdk_fail_closed_responses.py @@ -0,0 +1,245 @@ +"""Malformed synthetic gateway responses must not reach local tool bodies.""" + +from __future__ import annotations + +import inspect +import json +from unittest.mock import AsyncMock, patch + +import httpx +import pytest + +from acr.gateway.models import EvaluateResponse +from acr.pillar2_policy.models import PolicyDecision, PolicyEvaluationResult +from acr.sdk import ( + ACRClient, + ACRDeniedError, + ACREscalatedError, + ACRHTTPError, + ACRSDKError, + AsyncACRClient, + guard_async_tool, + guard_tool, +) + + +async def exercise(async_mode, status, payload, calls, *, execute_locally=True): + def tool(amount: int = 999): + calls.append(amount) + return amount + + def handler(request): + assert calls == [], "Authorization must precede the local side effect" + assert json.loads(request.content)["action"] == { + "tool_name": "refund", + "parameters": {"amount": 999}, + } + return httpx.Response(status, json=payload) + + client_type = AsyncACRClient if async_mode else ACRClient + client = client_type( + base_url="https://synthetic.invalid", transport=httpx.MockTransport(handler) + ) + try: + session = client.create_agent_session("synthetic-agent", "synthetic-token") + guard = guard_async_tool if async_mode else guard_tool + wrapped = guard( + tool, session=session, tool_name="refund", execute_locally_on_allow=execute_locally + ) + result = wrapped() + return await result if inspect.isawaitable(result) else result + finally: + if async_mode: + await client.aclose() + else: + client.close() + + +@pytest.mark.parametrize("async_mode", [False, True], ids=["sync", "async"]) +@pytest.mark.parametrize("status", [202, 403, 500, 503]) +@pytest.mark.parametrize("decision", ["allow", "modify"]) +async def test_non_success_status_cannot_authorize_local_effect(async_mode, status, decision): + calls = [] + payload = {"decision": decision} + if decision == "modify": + payload["modified_action"] = {"tool_name": "refund", "parameters": {"amount": 25}} + with pytest.raises(ACRHTTPError, match="cannot authorize execution"): + await exercise(async_mode, status, payload, calls) + assert calls == [] + + +@pytest.mark.parametrize("async_mode", [False, True], ids=["sync", "async"]) +@pytest.mark.parametrize("execute_locally", [False, True]) +@pytest.mark.parametrize( + "action,message", + [ + (None, "explicit modified action"), + ({"tool_name": "other", "parameters": {"amount": 25}}, "requested tool"), + ({"tool_name": "refund"}, "explicit parameters"), + ], +) +async def test_incomplete_or_mismatched_modify_cannot_execute( + async_mode, execute_locally, action, message +): + calls = [] + payload = {"decision": "modify", "execution_result": {"synthetic": True}} + if action is not None: + payload["modified_action"] = action + with pytest.raises(ACRSDKError, match=message): + await exercise(async_mode, 200, payload, calls, execute_locally=execute_locally) + assert calls == [] + + +@pytest.mark.parametrize("async_mode", [False, True], ids=["sync", "async"]) +@pytest.mark.parametrize( + "status,payload,expected,error", + [ + (200, {"decision": "allow"}, [999], None), + (200, {"decision": "deny"}, [], ACRDeniedError), + (403, {"decision": "deny"}, [], ACRDeniedError), + (500, {"decision": "deny"}, [], ACRDeniedError), + (503, {"decision": "deny"}, [], ACRDeniedError), + (200, {"decision": "escalate"}, [], ACREscalatedError), + (202, {"decision": "escalate"}, [], ACREscalatedError), + ( + 200, + { + "decision": "modify", + "modified_action": {"tool_name": "refund", "parameters": {"amount": 25}}, + }, + [25], + None, + ), + ( + 200, + { + "decision": "modify", + "modified_action": {"tool_name": "refund", "parameters": {"amount": 999}}, + }, + [999], + None, + ), + ], +) +async def test_valid_decisions_preserve_exact_local_effects( + async_mode, status, payload, expected, error +): + calls = [] + if error: + with pytest.raises(error): + await exercise(async_mode, status, payload, calls) + else: + result = await exercise(async_mode, status, payload, calls) + assert result == expected[0] + assert calls == expected + + +@pytest.mark.parametrize("async_mode", [False, True], ids=["sync", "async"]) +async def test_explicit_empty_modified_payload_and_guard_alias_remain_valid(async_mode): + calls = [] + result = await exercise( + async_mode, + 200, + {"decision": "modify", "modified_action": {"tool_name": "refund", "parameters": {}}}, + calls, + ) + assert calls == [999] + assert result == 999 + + +@pytest.mark.parametrize("async_mode", [False, True], ids=["sync", "async"]) +@pytest.mark.parametrize( + "payload", + [ + {"decision": "modify"}, + {"decision": "modify", "modified_action": None}, + {"decision": "modify", "modified_action": {"tool_name": "refund"}}, + { + "decision": "modify", + "modified_action": {"tool_name": "other", "parameters": {"amount": 25}}, + }, + ], +) +async def test_raw_client_rejects_incomplete_or_mismatched_transformation(async_mode, payload): + client_type = AsyncACRClient if async_mode else ACRClient + client = client_type( + base_url="https://synthetic.invalid", + transport=httpx.MockTransport(lambda request: httpx.Response(200, json=payload)), + ) + try: + with pytest.raises(ACRHTTPError): + result = client.evaluate_action( + agent_id="synthetic-agent", + access_token="synthetic-token", + tool_name="refund", + parameters={"amount": 999}, + ) + if inspect.isawaitable(result): + await result + finally: + if async_mode: + await client.aclose() + else: + client.close() + + +@pytest.mark.parametrize("async_mode", [False, True], ids=["sync", "async"]) +@pytest.mark.parametrize( + "action", + [None, {"tool_name": "tool"}, {"tool_name": "other", "parameters": {"amount": 25}}], +) +async def test_custom_session_cannot_bypass_guard_transformation_checks(async_mode, action): + calls = [] + response = EvaluateResponse.model_validate({"decision": "modify", "modified_action": action}) + + class SyncSession: + def evaluate_action(self, **kwargs): + return response + + class AsyncSession: + async def evaluate_action(self, **kwargs): + return response + + def tool(amount=999): + calls.append(amount) + + guard = guard_async_tool if async_mode else guard_tool + wrapped = guard(tool, session=AsyncSession() if async_mode else SyncSession()) + with pytest.raises(ACRSDKError): + result = wrapped() + if inspect.isawaitable(result): + await result + assert calls == [] + + +@pytest.mark.parametrize( + "policy_action,expected", + [ + ({"description": "Reviewed by policy"}, "C-001"), + ({"parameters": {"customer_id": "C-002"}}, "C-002"), + ], +) +async def test_sdk_accepts_actual_gateway_modify_response( + async_client, sample_agent, policy_action, expected +): + """Real router/SDK, isolated SQLite and mocked policy/Redis/auth fixtures.""" + policy = PolicyEvaluationResult( + final_decision="modify", + decisions=[PolicyDecision(policy_id="synthetic-policy", decision="modify")], + modified_action=policy_action, + ) + calls = [] + + async def query_customer_db(customer_id): + calls.append(customer_id) + return customer_id + + sdk = AsyncACRClient(base_url="http://test", client=async_client) + session = sdk.create_agent_session(sample_agent.agent_id, "synthetic-token") + guarded = guard_async_tool(query_customer_db, session=session) + with ( + patch("acr.gateway.router.evaluate_policy", new_callable=AsyncMock, return_value=policy), + patch("acr.gateway.router._persist_policy_decisions", new_callable=AsyncMock), + ): + assert await guarded(customer_id="C-001") == expected + assert calls == [expected]