Skip to content

fix: copy in-memory push configs so callers cannot mutate stored webhooks #1218

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
anxkhn wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: copy in-memory push configs so callers cannot mutate stored webhooks #1218

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
24 changes: 18 additions & 6 deletions src/a2a/server/tasks/inmemory_push_notification_config_store.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,14 @@
logger = logging.getLogger(__name__)


def _copy_config(
notification_config: TaskPushNotificationConfig,
) -> TaskPushNotificationConfig:
stored = TaskPushNotificationConfig()
stored.CopyFrom(notification_config)
return stored


class InMemoryPushNotificationConfigStore(PushNotificationConfigStore):
"""In-memory implementation of PushNotificationConfigStore interface.

Expand Down Expand Up @@ -48,16 +56,18 @@ async def set_info(
if task_id not in owner_infos:
owner_infos[task_id] = []

if not notification_config.id:
notification_config.id = task_id
Comment on lines -51 to -52

@mykytanetipa mykytanetipa Aug 30, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this leads to spec violation for on_create_task_push_notification_config (3.1.7: "Outputs: - TaskPushNotificationConfig: Created configuration with assigned ID")

on_create_task_push_notification_config is expected to set id for TaskPushNotificationConfig response if not provided by caller.

stored = TaskPushNotificationConfig()
stored.CopyFrom(notification_config)
Comment on lines +59 to +60

@mykytanetipa mykytanetipa Aug 30, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider using _copy_config for consistency

if not stored.id:
stored.id = task_id

# Remove existing config with the same ID
for config in owner_infos[task_id]:
if config.id == notification_config.id:
if config.id == stored.id:
owner_infos[task_id].remove(config)
break

owner_infos[task_id].append(notification_config)
owner_infos[task_id].append(stored)
logger.debug(
'Push notification config for task %s with config id %s for owner %s saved/updated.',
task_id,
Expand All @@ -77,7 +87,7 @@ async def get_info(
owner = self.owner_resolver(context)
with self.lock:
owner_infos = self._get_owner_push_notification_infos(owner)
return list(owner_infos.get(task_id, []))
return [_copy_config(item) for item in owner_infos.get(task_id, [])]

async def get_info_for_dispatch(
self,
Expand All @@ -90,7 +100,9 @@ async def get_info_for_dispatch(
with self.lock:
results: list[TaskPushNotificationConfig] = []
for all_configs in self._push_notification_infos.values():
results.extend(all_configs.get(task_id, []))
results.extend(
_copy_config(item) for item in all_configs.get(task_id, [])
)
return results

async def delete_info(
Expand Down
15 changes: 15 additions & 0 deletions tests/server/tasks/test_inmemory_push_notifications.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,21 @@ async def test_set_info_adds_new_config(self) -> None:
)
self.assertEqual(retrieved, [config])

async def test_set_info_copies_caller_and_returned_config(self) -> None:
task_id = 'task_copy'
config = _create_sample_push_config(url='http://orig.url/callback')
await self.config_store.set_info(task_id, config, MINIMAL_CALL_CONTEXT)

config.url = 'http://mutated.url/callback'
retrieved = await self.config_store.get_info(
task_id, MINIMAL_CALL_CONTEXT
)
self.assertEqual(retrieved[0].url, 'http://orig.url/callback')

retrieved[0].url = 'http://got.url/callback'
again = await self.config_store.get_info(task_id, MINIMAL_CALL_CONTEXT)
self.assertEqual(again[0].url, 'http://orig.url/callback')

async def test_set_info_appends_to_existing_config(self) -> None:
task_id = 'task_update'
initial_config = _create_sample_push_config(
Expand Down
Loading