From b1820c651da2a32df96859c9f3f55db3fdf2d8a9 Mon Sep 17 00:00:00 2001 From: yazhog Date: Mon, 30 Mar 2026 15:56:45 +0300 Subject: [PATCH] Fix RemnaWave webhook deletion race --- app/services/remnawave_webhook_service.py | 86 ++++++++++++++ app/services/user_service.py | 5 + app/webserver/remnawave_webhook.py | 17 ++- tests/webserver/test_remnawave_webhook.py | 130 ++++++++++++++++++++++ 4 files changed, 232 insertions(+), 6 deletions(-) create mode 100644 tests/webserver/test_remnawave_webhook.py diff --git a/app/services/remnawave_webhook_service.py b/app/services/remnawave_webhook_service.py index 50209afe..f576c888 100644 --- a/app/services/remnawave_webhook_service.py +++ b/app/services/remnawave_webhook_service.py @@ -129,6 +129,9 @@ class RemnaWaveWebhookService: # Key: subscription_id, Value: datetime of last recreation attempt. _recent_recreations: dict[int, datetime] = {} _RECREATION_GUARD_SECONDS: int = 120 # 2-minute cooldown + _intentional_panel_deletions_by_uuid: dict[str, datetime] = {} + _intentional_panel_deletions_by_telegram_id: dict[int, datetime] = {} + _INTENTIONAL_PANEL_DELETION_GUARD_SECONDS: int = 300 def __init__(self, bot: Bot) -> None: self.bot = bot @@ -169,12 +172,95 @@ class RemnaWaveWebhookService: """Check if the event is admin-scoped (no DB session needed).""" return event_name in self._admin_handlers + @classmethod + def _prune_intentional_panel_deletions(cls) -> None: + if not cls._intentional_panel_deletions_by_uuid and not cls._intentional_panel_deletions_by_telegram_id: + return + + now = datetime.now(UTC) + uuid_keys = [ + key + for key, created_at in cls._intentional_panel_deletions_by_uuid.items() + if (now - created_at).total_seconds() >= cls._INTENTIONAL_PANEL_DELETION_GUARD_SECONDS + ] + for key in uuid_keys: + del cls._intentional_panel_deletions_by_uuid[key] + + telegram_keys = [ + key + for key, created_at in cls._intentional_panel_deletions_by_telegram_id.items() + if (now - created_at).total_seconds() >= cls._INTENTIONAL_PANEL_DELETION_GUARD_SECONDS + ] + for key in telegram_keys: + del cls._intentional_panel_deletions_by_telegram_id[key] + + @classmethod + def mark_intentional_panel_deletion( + cls, + *, + panel_uuids: list[str] | None = None, + telegram_id: int | None = None, + ) -> None: + cls._prune_intentional_panel_deletions() + now = datetime.now(UTC) + + for panel_uuid in panel_uuids or []: + normalized = (panel_uuid or '').strip() + if normalized: + cls._intentional_panel_deletions_by_uuid[normalized] = now + + if telegram_id: + cls._intentional_panel_deletions_by_telegram_id[int(telegram_id)] = now + + @classmethod + def _is_intentional_panel_deletion_event(cls, data: dict[str, Any]) -> bool: + cls._prune_intentional_panel_deletions() + + candidate_uuids: list[str] = [] + candidate_telegram_ids: list[int] = [] + + for value in (data.get('uuid'), data.get('userUuid')): + if value: + candidate_uuids.append(str(value).strip()) + + telegram_id = data.get('telegramId') + if telegram_id: + try: + candidate_telegram_ids.append(int(telegram_id)) + except (TypeError, ValueError): + pass + + nested_user = data.get('user') + if isinstance(nested_user, dict): + nested_uuid = nested_user.get('uuid') + if nested_uuid: + candidate_uuids.append(str(nested_uuid).strip()) + + nested_tid = nested_user.get('telegramId') + if nested_tid: + try: + candidate_telegram_ids.append(int(nested_tid)) + except (TypeError, ValueError): + pass + + return any(uuid in cls._intentional_panel_deletions_by_uuid for uuid in candidate_uuids) or any( + telegram_id in cls._intentional_panel_deletions_by_telegram_id for telegram_id in candidate_telegram_ids + ) + async def process_event(self, db: AsyncSession | None, event_name: str, data: dict) -> bool: """Route event to the appropriate handler. Returns True if the event was processed, False if skipped/unknown. db may be None for admin events that don't require database access. """ + if event_name == 'user.deleted' and self._is_intentional_panel_deletion_event(data): + logger.info( + 'RemnaWave webhook: skipping intentional admin-triggered user.deleted event', + telegram_id=data.get('telegramId'), + remnawave_uuid=data.get('uuid') or data.get('userUuid'), + ) + return True + # Check admin-scoped handlers (no DB needed) if event_name in self._admin_handlers: return await self._process_admin_event(event_name, data) diff --git a/app/services/user_service.py b/app/services/user_service.py index d0ac4a46..a94c0097 100644 --- a/app/services/user_service.py +++ b/app/services/user_service.py @@ -820,10 +820,15 @@ class UserService: for panel_uuid in panel_uuids: try: from app.services.remnawave_service import RemnaWaveService + from app.services.remnawave_webhook_service import RemnaWaveWebhookService remnawave_service = RemnaWaveService() if delete_mode == 'delete': + RemnaWaveWebhookService.mark_intentional_panel_deletion( + panel_uuids=[panel_uuid], + telegram_id=int(user.telegram_id) if user.telegram_id else None, + ) async with remnawave_service.get_api_client() as api: delete_success = await api.delete_user(panel_uuid) if delete_success: diff --git a/app/webserver/remnawave_webhook.py b/app/webserver/remnawave_webhook.py index 2c4aa851..4e663347 100644 --- a/app/webserver/remnawave_webhook.py +++ b/app/webserver/remnawave_webhook.py @@ -97,18 +97,23 @@ def create_remnawave_webhook_router(bot: Bot) -> APIRouter: status_code=status.HTTP_400_BAD_REQUEST, ) - # Extract and validate event info - scope = payload.get('scope', '') - event = payload.get('event', '') + # Extract and validate event info. Recent RemnaWave payloads send only + # the fully-qualified event name (for example "user.modified") without + # a separate top-level scope field. + event = str(payload.get('event', '') or '').strip() + scope = str(payload.get('scope', '') or '').strip() data = payload.get('data') - if not scope or not event: - logger.warning('RemnaWave webhook: missing scope or event') + if not event: + logger.warning('RemnaWave webhook: missing event') return JSONResponse( - {'status': 'error', 'reason': 'missing_scope_or_event'}, + {'status': 'error', 'reason': 'missing_event'}, status_code=status.HTTP_400_BAD_REQUEST, ) + if not scope and '.' in event: + scope = event.split('.', 1)[0] + if not isinstance(data, dict): data = {} diff --git a/tests/webserver/test_remnawave_webhook.py b/tests/webserver/test_remnawave_webhook.py new file mode 100644 index 00000000..33bdef5e --- /dev/null +++ b/tests/webserver/test_remnawave_webhook.py @@ -0,0 +1,130 @@ +import hashlib +import hmac +import json +from types import SimpleNamespace +from typing import Any +from unittest.mock import AsyncMock + +import pytest +from starlette.requests import Request + +from app.config import settings +from app.services.remnawave_webhook_service import RemnaWaveWebhookService +from app.webserver.remnawave_webhook import create_remnawave_webhook_router + + +@pytest.fixture(autouse=True) +def reset_remnawave_webhook_settings(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setattr(settings, 'REMNAWAVE_WEBHOOK_ENABLED', True, raising=False) + monkeypatch.setattr(settings, 'REMNAWAVE_WEBHOOK_PATH', '/remnawave-webhook', raising=False) + monkeypatch.setattr( + settings, + 'REMNAWAVE_WEBHOOK_SECRET', + '0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef', + raising=False, + ) + RemnaWaveWebhookService._intentional_panel_deletions_by_uuid.clear() + RemnaWaveWebhookService._intentional_panel_deletions_by_telegram_id.clear() + + +def _get_route(router, path: str, method: str = 'POST'): + for route in router.routes: + if getattr(route, 'path', '') == path and method in getattr(route, 'methods', set()): + return route + raise AssertionError(f'Route {path} with method {method} not found') + + +def _build_request(path: str, body: bytes, headers: dict[str, str] | None = None) -> Request: + scope = { + 'type': 'http', + 'asgi': {'version': '3.0'}, + 'method': 'POST', + 'path': path, + 'headers': [(k.lower().encode('latin-1'), v.encode('latin-1')) for k, v in (headers or {}).items()], + } + + async def receive() -> dict[str, Any]: + return {'type': 'http.request', 'body': body, 'more_body': False} + + return Request(scope, receive) + + +def _signature(body: bytes) -> str: + secret = settings.REMNAWAVE_WEBHOOK_SECRET or '' + return hmac.new(secret.encode('utf-8'), body, hashlib.sha256).hexdigest() + + +@pytest.mark.anyio +async def test_remnawave_webhook_accepts_event_without_scope(monkeypatch: pytest.MonkeyPatch) -> None: + bot = AsyncMock() + process_event = AsyncMock(return_value=True) + service = SimpleNamespace( + process_event=process_event, + is_admin_event=lambda _event_name: True, + ) + + monkeypatch.setattr( + 'app.webserver.remnawave_webhook.RemnaWaveWebhookService', + lambda _bot: service, + ) + + payload = { + 'event': 'user.modified', + 'data': {'uuid': 'user-123'}, + 'timestamp': '2026-03-30T12:00:00.000Z', + } + raw_body = json.dumps(payload).encode('utf-8') + + router = create_remnawave_webhook_router(bot) + path = settings.REMNAWAVE_WEBHOOK_PATH + route = _get_route(router, path) + request = _build_request( + path, + raw_body, + headers={'X-Remnawave-Signature': _signature(raw_body)}, + ) + + response = await route.endpoint(request) + + assert response.status_code == 200 + process_event.assert_awaited_once_with(None, 'user.modified', {'uuid': 'user-123'}) + + +@pytest.mark.anyio +async def test_remnawave_webhook_rejects_payload_without_event() -> None: + bot = AsyncMock() + payload = {'data': {'uuid': 'user-123'}} + raw_body = json.dumps(payload).encode('utf-8') + + router = create_remnawave_webhook_router(bot) + path = settings.REMNAWAVE_WEBHOOK_PATH + route = _get_route(router, path) + request = _build_request( + path, + raw_body, + headers={'X-Remnawave-Signature': _signature(raw_body)}, + ) + + response = await route.endpoint(request) + + assert response.status_code == 400 + assert json.loads(response.body.decode('utf-8')) == {'status': 'error', 'reason': 'missing_event'} + + +@pytest.mark.anyio +async def test_process_event_skips_intentional_admin_user_deleted() -> None: + bot = AsyncMock() + service = RemnaWaveWebhookService(bot) + + RemnaWaveWebhookService.mark_intentional_panel_deletion( + panel_uuids=['panel-user-123'], + telegram_id=8368498066, + ) + + processed = await service.process_event( + None, + 'user.deleted', + {'uuid': 'panel-user-123', 'telegramId': 8368498066}, + ) + + assert processed is True