Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion src/a2a/server/request_handlers/grpc_handler.py
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,9 @@ async def _handle_unary(
result = await handler_func(server_context)
except A2AError as e:
await self.abort_context(e, context)
except Exception:
logger.exception('Unhandled exception in gRPC handler')
await self.abort_context(types.InternalError(), context)
else:
return result
return default_response
Expand All @@ -153,6 +156,9 @@ async def _handle_stream(
yield item
except A2AError as e:
await self.abort_context(e, context)
except Exception:
logger.exception('Unhandled exception in gRPC handler')
await self.abort_context(types.InternalError(), context)

async def SendMessage(
self,
Expand Down Expand Up @@ -413,9 +419,13 @@ async def abort_context(
context.set_trailing_metadata(tuple(new_metadata))
await context.abort(rich_status.code, rich_status.details)
else:
logger.error(
'Unknown error type during request handling',
exc_info=error,
)
await context.abort(
grpc.StatusCode.UNKNOWN,
f'Unknown error type: {error}',
'Unknown error',
)

def _build_call_context(
Expand Down
20 changes: 10 additions & 10 deletions src/a2a/server/routes/jsonrpc_dispatcher.py
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,9 @@ def _generate_error_response(
A `JSONResponse` object formatted as a JSON-RPC error response.
"""
if not isinstance(error, A2AError | JSONRPCError):
error = InternalError(message=str(error))
# Never leak internal exception details to the client; the
# original error was logged by the caller.
error = InternalError()

response_data = build_error_response(request_id, error)
error_info = response_data.get('error', {})
Expand Down Expand Up @@ -251,11 +253,11 @@ async def handle_requests(self, request: Request) -> Response: # noqa: PLR0911,
message="Invalid request: 'jsonrpc' must be exactly '2.0'"
),
)
except Exception as e:
except Exception:
logger.exception('Failed to validate base JSON-RPC request')
return self._generate_error_response(
request_id,
InvalidRequestError(data=str(e)),
InvalidRequestError(),
)

# 2) Route by method name; unknown -> -32601, known -> validate params (-32602 on failure)
Expand Down Expand Up @@ -289,11 +291,11 @@ async def handle_requests(self, request: Request) -> Response: # noqa: PLR0911,
# Parse the params field into the proto message type
params = body.get('params', {})
specific_request = ParseDict(params, model_class())
except Exception as e:
except Exception:
logger.exception('Failed to parse request params')
return self._generate_error_response(
request_id,
InvalidParamsError(data=str(e)),
InvalidParamsError(),
)

# 3) Build call context and wrap the request for downstream handling
Expand Down Expand Up @@ -335,11 +337,9 @@ async def handle_requests(self, request: Request) -> Response: # noqa: PLR0911,
raise e
except A2AError as e:
return self._generate_error_response(request_id, e)
except Exception as e:
except Exception:
logger.exception('Unhandled exception')
return self._generate_error_response(
request_id, InternalError(message=str(e))
)
return self._generate_error_response(request_id, InternalError())

@validate_version(constants.PROTOCOL_VERSION_1_0)
async def _process_streaming_request(
Expand Down Expand Up @@ -585,7 +585,7 @@ async def event_generator(
rpc_error: A2AError | JSONRPCError = (
e
if isinstance(e, A2AError | JSONRPCError)
else InternalError(message=str(e))
else InternalError()
)
error_response = build_error_response(
context.state.get('request_id'), rpc_error
Expand Down
23 changes: 16 additions & 7 deletions tests/integration/test_scenarios.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
from a2a.auth.user import User
from a2a.client.client import ClientConfig
from a2a.client.client_factory import ClientFactory
from a2a.client.errors import A2AClientError
from a2a.helpers.proto_helpers import new_task_from_user_message
from a2a.server.agent_execution import AgentExecutor, RequestContext
from a2a.server.context import ServerCallContext
Expand Down Expand Up @@ -49,6 +48,7 @@
)
from a2a.utils import TransportProtocol
from a2a.utils.errors import (
A2AError,
InvalidAgentResponseError,
InvalidParamsError,
TaskNotCancelableError,
Expand Down Expand Up @@ -469,7 +469,9 @@ async def cancel(
)

# TODO: Is it correct error code ?
with pytest.raises(A2AClientError, match='TEST_ERROR_IN_EXECUTE'):
# Agent exceptions are sanitized server-side (internal details are
# logged, not exposed to clients); clients receive the generic error.
with pytest.raises(A2AError, match='Internal error'):
async for _ in client.send_message(
SendMessageRequest(
message=msg,
Expand Down Expand Up @@ -548,7 +550,9 @@ async def release_agent():

tasks.append(asyncio.create_task(release_agent()))

with pytest.raises(A2AClientError, match='TEST_ERROR_IN_EXECUTE'):
# Agent exceptions are sanitized server-side (internal details are
# logged, not exposed to clients); clients receive the generic error.
with pytest.raises(A2AError, match='Internal error'):
async for _ in it:
pass

Expand Down Expand Up @@ -610,7 +614,9 @@ async def cancel(

await asyncio.wait_for(started_event.wait(), timeout=1.0)

with pytest.raises(A2AClientError, match='TEST_ERROR_IN_CANCEL'):
# Agent exceptions are sanitized server-side; clients receive the
# generic error.
with pytest.raises(A2AError, match='Internal error'):
await client.cancel_task(CancelTaskRequest(id=task_id))

(task,) = (await client.list_tasks(ListTasksRequest())).tasks
Expand Down Expand Up @@ -678,7 +684,9 @@ async def consume_events():
with pytest.raises(asyncio.TimeoutError):
await asyncio.wait_for(consume_task, timeout=0.1)
else:
with pytest.raises(A2AClientError, match='TEST_ERROR_IN_EXECUTE'):
# Agent exceptions are sanitized server-side (internal details are
# logged, not exposed to clients); clients receive the generic error.
with pytest.raises(A2AError, match='Internal error'):
await consume_task

(task,) = (await client.list_tasks(ListTasksRequest())).tasks
Expand Down Expand Up @@ -1019,8 +1027,9 @@ async def cancel(

# Verify that both calls for clients finished.
if use_legacy and not streaming:
# Legacy handler fails on first execution.
with pytest.raises(A2AClientError, match='NoTaskQueue'):
# Legacy handler fails on first execution; the failure is
# sanitized server-side (internal details are logged, not exposed).
with pytest.raises(A2AError, match='Internal error'):
await task1
else:
await task1
Expand Down
46 changes: 46 additions & 0 deletions tests/server/request_handlers/test_grpc_handler.py
Original file line number Diff line number Diff line change
Expand Up @@ -746,3 +746,49 @@ async def mock_stream(*args, **kwargs):
server_context = call_args[0][1]
assert isinstance(server_context, ServerCallContext)
assert server_context.tenant == ''


@pytest.mark.asyncio
async def test_unhandled_exception_is_sanitized(
grpc_handler: GrpcHandler,
mock_request_handler: AsyncMock,
mock_grpc_context: AsyncMock,
) -> None:
"""A non-A2A exception must not leak its message to the client (BUG-46)."""
mock_request_handler.on_get_task.side_effect = RuntimeError(
'internal detail: /secret/path'
)
request_proto = a2a_pb2.GetTaskRequest(id='any')

await grpc_handler.GetTask(request_proto, mock_grpc_context)

mock_grpc_context.abort.assert_awaited_once()
call_args, _ = mock_grpc_context.abort.call_args
assert call_args[0] == grpc.StatusCode.INTERNAL
assert 'internal detail' not in call_args[1]
assert 'INTERNAL' in call_args[1] or 'Internal error' in call_args[1]


@pytest.mark.asyncio
async def test_unknown_a2a_error_type_is_sanitized(
grpc_handler: GrpcHandler,
mock_request_handler: AsyncMock,
mock_grpc_context: AsyncMock,
) -> None:
"""An A2AError outside the mapping must not leak details (BUG-46)."""
from a2a.utils.errors import A2AError

class CustomError(A2AError):
message = 'custom'

mock_request_handler.on_get_task.side_effect = CustomError(
'sensitive internals here'
)
request_proto = a2a_pb2.GetTaskRequest(id='any')

await grpc_handler.GetTask(request_proto, mock_grpc_context)

mock_grpc_context.abort.assert_awaited_once()
call_args, _ = mock_grpc_context.abort.call_args
assert call_args[0] == grpc.StatusCode.UNKNOWN
assert 'sensitive internals' not in call_args[1]
54 changes: 54 additions & 0 deletions tests/server/routes/test_jsonrpc_dispatcher.py
Original file line number Diff line number Diff line change
Expand Up @@ -653,3 +653,57 @@ async def stream_generator():

if __name__ == '__main__':
pytest.main([__file__])


# --- Error sanitization (BUG-12 / BUG-46) ---


class TestErrorSanitization:
def test_unhandled_exception_does_not_leak(self, client, mock_handler):
"""Non-A2A exceptions must not leak their message to the client."""
mock_handler.on_get_task.side_effect = RuntimeError(
'internal detail: /secret/path'
)
response = client.post(
'/',
json={
'jsonrpc': '2.0',
'id': '1',
'method': 'GetTask',
'params': {'id': 'task1'},
},
)
data = response.json()
assert data['error']['code'] == -32603 # InternalError
assert data['error']['message'] == 'Internal error'
assert 'internal detail' not in response.text

def test_malformed_params_does_not_leak(self, client):
"""Parse failures must not leak the raw parse error to the client."""
response = client.post(
'/',
json={
'jsonrpc': '2.0',
'id': '1',
'method': 'GetTask',
# id must be a string; a dict fails proto parsing.
'params': {'id': {'nested': 'invalid'}},
},
)
data = response.json()
assert data['error']['code'] == -32602 # InvalidParamsError
assert 'nested' not in response.text

def test_invalid_base_request_does_not_leak(self, client):
"""Base JSON-RPC validation failures must not leak details."""
response = client.post(
'/',
json={
'jsonrpc': '2.0',
'id': '1',
'method': 'GetTask',
'params': 'not-a-dict',
},
)
data = response.json()
assert data['error']['code'] == -32600 # InvalidRequestError
6 changes: 4 additions & 2 deletions tests/server/test_integration.py
Original file line number Diff line number Diff line change
Expand Up @@ -898,7 +898,7 @@ def test_validation_error(client: TestClient):


def test_unhandled_exception(client: TestClient, handler: mock.AsyncMock):
"""Test handling unhandled exception."""
"""Test handling unhandled exception without leaking internal details."""
handler.on_get_task.side_effect = Exception('Unexpected error')

response = client.post(
Expand All @@ -914,7 +914,9 @@ def test_unhandled_exception(client: TestClient, handler: mock.AsyncMock):
data = response.json()
assert 'error' in data
assert data['error']['code'] == InternalError().code
assert 'Unexpected error' in data['error']['message']
# The internal exception message must not leak to the client.
assert data['error']['message'] == 'Internal error'
assert 'Unexpected error' not in data['error']['message']


def test_get_method_to_rpc_endpoint(client: TestClient):
Expand Down
Loading