Merge pull request #99 from GeiserX/fix/security-hardening-round2

fix: security hardening round 2 (v7.4.1)

Author: GeiserX

src/__init__.py

@@ -2,4 +2,4 @@
 Telegram Backup Automation - Main Package
 """
 
-__version__ = "7.4.0"
+__version__ = "7.4.1"

src/db/adapter.py

@@ -783,22 +783,24 @@ async def insert_reactions(self, message_id: int, chat_id: int, reactions: list[
                     await session.flush()  # Flush each to catch errors early
                 except Exception as e:
                     if "duplicate key" in str(e).lower() or "unique" in str(e).lower():
-                        # Sequence out of sync - reset and retry
-                        logger.warning("Reactions sequence out of sync, resetting...")
+                        # Sequence out of sync — rollback undoes ALL flushed inserts
+                        logger.warning("Reactions sequence out of sync, resetting and retrying all...")
                         await session.rollback()
                         await self._reset_reactions_sequence()
-                        # Retry the insert
+                        # Retry ALL reactions in a fresh transaction
                         async with self.db_manager.async_session_factory() as retry_session:
-                            r = Reaction(
-                                message_id=message_id,
-                                chat_id=chat_id,
-                                emoji=reaction["emoji"],
-                                user_id=reaction.get("user_id"),
-                                count=reaction.get("count", 1),
-                            )
-                            retry_session.add(r)
+                            for r_data in reactions:
+                                retry_session.add(
+                                    Reaction(
+                                        message_id=message_id,
+                                        chat_id=chat_id,
+                                        emoji=r_data["emoji"],
+                                        user_id=r_data.get("user_id"),
+                                        count=r_data.get("count", 1),
+                                    )
+                                )
                             await retry_session.commit()
-                        return  # Exit after recovery
+                        return
                     raise
 
             await session.commit()

src/realtime.py

@@ -130,12 +130,19 @@ async def _notify_http(self, payload: dict):
         if not self._http_endpoint:
             return
 
+        headers: dict[str, str] = {}
+        push_secret = os.getenv("INTERNAL_PUSH_SECRET")
+        if push_secret:
+            headers["Authorization"] = f"Bearer {push_secret}"
+
         try:
             import aiohttp
 
             async with (
                 aiohttp.ClientSession() as session,
-                session.post(self._http_endpoint, json=payload, timeout=aiohttp.ClientTimeout(total=5)) as response,
+                session.post(
+                    self._http_endpoint, json=payload, headers=headers, timeout=aiohttp.ClientTimeout(total=5)
+                ) as response,
             ):
                 if response.status != 200:
                     logger.warning(f"HTTP notification returned {response.status}")
@@ -145,7 +152,7 @@ async def _notify_http(self, payload: dict):
                 import httpx
 
                 async with httpx.AsyncClient() as client:
-                    await client.post(self._http_endpoint, json=payload, timeout=5)
+                    await client.post(self._http_endpoint, json=payload, headers=headers, timeout=5)
             except ImportError:
                 logger.warning("Neither aiohttp nor httpx available for HTTP notifications")
         except Exception as e:

src/web/main.py

@@ -714,12 +714,23 @@ async def serve_thumbnail(size: int, folder: str, filename: str, user: UserConte
     # Chat-level access check
     user_chat_ids = get_user_chat_ids(user)
     if user_chat_ids is not None:
-        try:
-            media_chat_id = int(folder.split("/")[0])
-            if media_chat_id not in user_chat_ids:
+        folder_parts = folder.split("/")
+        if folder_parts[0] == "avatars":
+            # Avatar thumbnail: folder=avatars/{users|chats}, filename={chat_id}_{photo_id}.jpg
+            name = filename.rsplit(".", 1)[0] if "." in filename else filename
+            try:
+                avatar_chat_id = int(name.split("_")[0])
+                if avatar_chat_id not in user_chat_ids:
+                    raise HTTPException(status_code=403, detail="Access denied")
+            except ValueError:
                 raise HTTPException(status_code=403, detail="Access denied")
-        except ValueError:
-            pass
+        else:
+            try:
+                media_chat_id = int(folder_parts[0])
+                if media_chat_id not in user_chat_ids:
+                    raise HTTPException(status_code=403, detail="Access denied")
+            except ValueError:
+                pass
 
     from .thumbnails import ensure_thumbnail
 
@@ -758,13 +769,24 @@ async def serve_media(path: str, download: int = Query(0), user: UserContext = D
     user_chat_ids = get_user_chat_ids(user)
     if user_chat_ids is not None:
         parts = path.split("/")
-        if len(parts) >= 2 and parts[0] != "avatars":
-            try:
-                media_chat_id = int(parts[0])
-                if media_chat_id not in user_chat_ids:
+        if len(parts) >= 2:
+            if parts[0] == "avatars" and len(parts) >= 3:
+                # Avatar path: avatars/{users|chats}/{chat_id}_{photo_id}.jpg
+                # Extract chat_id from filename to enforce per-chat ACL
+                name = parts[2].rsplit(".", 1)[0] if "." in parts[2] else parts[2]
+                try:
+                    avatar_chat_id = int(name.split("_")[0])
+                    if avatar_chat_id not in user_chat_ids:
+                        raise HTTPException(status_code=403, detail="Access denied")
+                except ValueError:
                     raise HTTPException(status_code=403, detail="Access denied")
-            except ValueError:
-                pass
+            elif parts[0] != "avatars":
+                try:
+                    media_chat_id = int(parts[0])
+                    if media_chat_id not in user_chat_ids:
+                        raise HTTPException(status_code=403, detail="Access denied")
+                except ValueError:
+                    pass
 
     if not resolved.is_file():
         raise HTTPException(status_code=404, detail="File not found")
@@ -1490,7 +1512,7 @@ async def push_unsubscribe(request: Request, user: UserContext = Depends(require
         if not endpoint:
             raise HTTPException(status_code=400, detail="Missing endpoint")
 
-        success = await push_manager.unsubscribe(endpoint)
+        success = await push_manager.unsubscribe(endpoint, username=user.username)
         return {"status": "unsubscribed" if success else "not_found"}
 
     except json.JSONDecodeError:
@@ -1517,13 +1539,15 @@ async def internal_push(request: Request):
     Access is restricted to loopback and private (RFC1918/Docker) IPs.
     Split-container SQLite setups use VIEWER_HOST/VIEWER_PORT to push
     from the backup container to the viewer container over Docker networks.
+
+    If INTERNAL_PUSH_SECRET is set, it must be provided as a bearer token.
+    This prevents co-tenant containers from spoofing live events.
     """
     import ipaddress
 
     client_host = request.client.host if request.client else None
 
     # Accept from loopback + private IPs (Docker internal, RFC1918)
-    # Split-container SQLite needs this for cross-container push
     is_allowed = False
     if client_host:
         try:
@@ -1536,6 +1560,14 @@ async def internal_push(request: Request):
         logger.warning(f"Rejected /internal/push from non-private IP: {client_host}")
         raise HTTPException(status_code=403, detail="Forbidden")
 
+    # Optional shared secret for multi-tenant Docker environments
+    push_secret = os.getenv("INTERNAL_PUSH_SECRET")
+    if push_secret:
+        auth_header = request.headers.get("Authorization", "")
+        if auth_header != f"Bearer {push_secret}":
+            logger.warning(f"Rejected /internal/push: invalid or missing secret from {client_host}")
+            raise HTTPException(status_code=403, detail="Forbidden")
+
     try:
         payload = await request.json()
         if realtime_listener:

src/web/push.py

@@ -173,17 +173,20 @@ async def subscribe(
             logger.error(f"Failed to store push subscription: {e}")
             return False
 
-    async def unsubscribe(self, endpoint: str) -> bool:
-        """Remove a push subscription."""
+    async def unsubscribe(self, endpoint: str, username: str | None = None) -> bool:
+        """Remove a push subscription. Scoped to the requesting user to prevent cross-user unsubscribe."""
         try:
-            from sqlalchemy import delete
+            from sqlalchemy import and_, delete
 
             from src.db.models import PushSubscription
 
             async with self.db.db_manager.async_session_factory() as session:
-                await session.execute(delete(PushSubscription).where(PushSubscription.endpoint == endpoint))
+                conditions = [PushSubscription.endpoint == endpoint]
+                if username:
+                    conditions.append(PushSubscription.username == username)
+                await session.execute(delete(PushSubscription).where(and_(*conditions)))
                 await session.commit()
-                logger.info(f"Push subscription removed: {endpoint[:50]}...")
+                logger.info(f"Push subscription removed for {username or 'anonymous'}: {endpoint[:50]}...")
                 return True
 
         except Exception as e: