-
-
Notifications
You must be signed in to change notification settings - Fork 19
fix(media): correct fallback logic, ACL bypass, and migration cleanup #169
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.
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
Changes from all commits
789c42b
8ed33fb
5c4fe8a
b8dfc76
50554f2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,4 +2,4 @@ | |
| Telegram Backup Automation - Main Package | ||
| """ | ||
|
|
||
| __version__ = "7.10.14" | ||
| __version__ = "7.10.15" | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -31,6 +31,7 @@ | |
| from ..config import Config | ||
| from ..db import DatabaseAdapter, close_database, get_db_manager, init_database | ||
| from ..realtime import RealtimeListener | ||
| from .media_utils import legacy_folder_alternates, legacy_marked_chat_ids | ||
|
|
||
| if TYPE_CHECKING: | ||
| from .push import PushNotificationManager | ||
|
|
@@ -821,8 +822,8 @@ def _enforce_media_acl(path: str, user: UserContext, *, thumbnail: bool = False) | |
| raise HTTPException(status_code=403, detail="Access denied") | ||
| if media_chat_id not in user_chat_ids: | ||
| # Legacy fallback: positive folder may correspond to negative marked ID | ||
| if media_chat_id > 0 and (-media_chat_id in user_chat_ids or -(1000000000000 + media_chat_id) in user_chat_ids): | ||
| pass | ||
| if media_chat_id > 0 and any(mid in user_chat_ids for mid in legacy_marked_chat_ids(media_chat_id)): | ||
| logger.debug("ACL legacy grant: positive folder mapped to allowed chat via marked-ID convention") | ||
| else: | ||
| raise HTTPException(status_code=403, detail="Access denied") | ||
|
|
||
|
|
@@ -885,7 +886,7 @@ async def serve_thumbnail(size: int, folder: str, filename: str, user: UserConte | |
| if user.no_download and not folder.startswith("avatars/"): | ||
| raise HTTPException(status_code=403, detail="Downloads disabled for this account") | ||
|
|
||
| # Chat-level access check | ||
| # Early ACL check on requested path (prevents existence leakage) | ||
| _enforce_media_acl(f"{folder}/{filename}", user, thumbnail=True) | ||
|
|
||
| from .thumbnails import ensure_thumbnail, resolve_cache_dir | ||
|
|
@@ -894,10 +895,15 @@ async def serve_thumbnail(size: int, folder: str, filename: str, user: UserConte | |
| if _thumb_cache_dir is None: | ||
| _thumb_cache_dir = resolve_cache_dir(_media_root) | ||
|
|
||
| thumb_path = await ensure_thumbnail(_media_root, size, folder, filename, cache_dir=_thumb_cache_dir) | ||
| if not thumb_path: | ||
| result = await ensure_thumbnail(_media_root, size, folder, filename, cache_dir=_thumb_cache_dir) | ||
| if not result: | ||
| raise HTTPException(status_code=404, detail="Thumbnail not available") | ||
|
|
||
| thumb_path, resolved_folder = result | ||
| # Secondary ACL on resolved path if it differs (prevents bypass via legacy fallback) | ||
| if resolved_folder != folder: | ||
| _enforce_media_acl(f"{resolved_folder}/{filename}", user, thumbnail=True) | ||
|
|
||
| return FileResponse(thumb_path, media_type="image/webp", headers={"Cache-Control": "public, max-age=86400"}) | ||
|
|
||
|
|
||
|
|
@@ -928,23 +934,20 @@ async def serve_media(path: str, download: int = Query(0), user: UserContext = D | |
| resolved = None | ||
| if len(parts) == 2: | ||
| folder, rest = parts | ||
| alt_folders = [] | ||
| if not folder.startswith("-"): | ||
| alt_folders = [f"-{folder}", f"-100{folder}"] | ||
| else: | ||
| alt_folders = [folder[1:]] | ||
| alt_folders = legacy_folder_alternates(folder) | ||
| for alt in alt_folders: | ||
| try: | ||
| resolved = (_media_root / alt / rest).resolve(strict=True) | ||
| logger.debug("Legacy fallback: served media via alternate folder resolution") | ||
| break | ||
| except OSError, ValueError: | ||
| except OSError, ValueError, RuntimeError: | ||
| continue | ||
| if resolved is None: | ||
| raise HTTPException(status_code=404, detail="File not found") | ||
|
Comment on lines
+937
to
946
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Guard alternate-folder generation for non-numeric folder names. Line 932 can raise Proposed fix- alt_folders = legacy_folder_alternates(folder)
+ try:
+ alt_folders = legacy_folder_alternates(folder)
+ except ValueError:
+ alt_folders = []🤖 Prompt for AI Agents |
||
| if not resolved.is_relative_to(_media_root): | ||
| raise HTTPException(status_code=403, detail="Access denied") | ||
|
|
||
| _enforce_media_acl(path, user) | ||
| _enforce_media_acl(str(resolved.relative_to(_media_root)), user) | ||
|
|
||
| if not resolved.is_file(): | ||
| raise HTTPException(status_code=404, detail="File not found") | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,56 @@ | ||
| """Shared utilities for legacy media path resolution. | ||
|
|
||
| Centralizes the Telegram marked-ID convention so it's defined once | ||
| and used consistently across serve_media, thumbnails, and ACL checks. | ||
| """ | ||
|
|
||
| CHANNEL_ID_OFFSET: int = 1_000_000_000_000 | ||
|
|
||
|
|
||
| def legacy_folder_alternates(folder: str) -> list[str]: | ||
| """Return alternate folder names for legacy positive/negative ID paths. | ||
|
|
||
| Forward (positive folder → possible negative marked IDs on disk): | ||
| "1234567890" → ["-1234567890", "-1001234567890"] | ||
|
|
||
| Reverse (negative folder → possible old positive folder on disk): | ||
| "-1234567890" → ["1234567890"] (basic group) | ||
| "-1001234567890" → ["1234567890"] (channel) | ||
| """ | ||
| try: | ||
| if not folder.startswith("-"): | ||
| folder_int = int(folder) | ||
| if folder_int <= 0: | ||
| return [] | ||
| return [f"-{folder}", str(-(CHANNEL_ID_OFFSET + folder_int))] | ||
| folder_int = int(folder) | ||
| except ValueError: | ||
| return [] | ||
| raw = -folder_int | ||
| if raw > CHANNEL_ID_OFFSET: | ||
| return [str(raw - CHANNEL_ID_OFFSET)] | ||
| return [str(raw)] | ||
|
|
||
|
|
||
| def legacy_marked_chat_ids(positive_id: int) -> list[int]: | ||
| """Return possible marked chat_ids for a legacy positive folder ID. | ||
|
|
||
| Used by ACL checks to determine if a user has access to a chat | ||
| referenced by its old positive folder name. | ||
| """ | ||
| return [-positive_id, -(CHANNEL_ID_OFFSET + positive_id)] | ||
|
|
||
|
|
||
| def derive_stale_folder(chat_id: int) -> str | None: | ||
| """Derive the old positive folder name from a marked chat_id. | ||
|
|
||
| Basic groups: chat_id = -X → old folder = "X" | ||
| Channels: chat_id = -(10^12 + X) → old folder = "X" | ||
| Users: chat_id > 0 → no mismatch possible, return None | ||
| """ | ||
| if chat_id >= 0: | ||
| return None | ||
| raw = -chat_id | ||
| if raw > CHANNEL_ID_OFFSET: | ||
| return str(raw - CHANNEL_ID_OFFSET) | ||
| return str(raw) |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -15,6 +15,8 @@ | |||||
|
|
||||||
| from PIL import Image | ||||||
|
|
||||||
| from .media_utils import legacy_folder_alternates | ||||||
|
|
||||||
| logger = logging.getLogger(__name__) | ||||||
|
|
||||||
| # Limit decompression to prevent pixel-bomb OOM attacks (~50 megapixels) | ||||||
|
|
@@ -87,11 +89,12 @@ | |||||
|
|
||||||
| async def ensure_thumbnail( | ||||||
| media_root: Path, size: int, folder: str, filename: str, *, cache_dir: Path | None = None | ||||||
| ) -> Path | None: | ||||||
| """Return the path to a cached thumbnail, generating it if needed. | ||||||
| ) -> tuple[Path, str] | None: | ||||||
| """Return (thumb_path, resolved_folder) or None. | ||||||
|
|
||||||
| Returns None when the request is invalid or generation fails. | ||||||
| Includes path traversal protection. | ||||||
| resolved_folder is the actual folder the source was found in (may differ | ||||||
| from the requested folder due to legacy ID fallback). Callers use this | ||||||
| for ACL enforcement on the resolved path. | ||||||
|
|
||||||
| When cache_dir is provided, thumbnails are written there instead of | ||||||
| under {media_root}/.thumbs/ — this supports read-only media volumes. | ||||||
|
|
@@ -120,28 +123,29 @@ | |||||
| if not dest.is_relative_to(thumbs_root): | ||||||
| return None | ||||||
|
|
||||||
| resolved_folder = folder | ||||||
|
|
||||||
| if dest.exists(): | ||||||
| return dest | ||||||
| return dest, resolved_folder | ||||||
|
|
||||||
| if not source.exists(): | ||||||
| # Legacy fallback: pre-v4.0.5 paths used positive IDs, disk uses negative marked IDs. | ||||||
| # Try alternate folder names: X→-X (basic group), X→-100X (channel/supergroup) | ||||||
| alt_folders = [] | ||||||
| if not folder.startswith("-"): | ||||||
| alt_folders = [f"-{folder}", f"-100{folder}"] | ||||||
| else: | ||||||
| alt_folders = [folder[1:]] | ||||||
| alt_folders = legacy_folder_alternates(folder) | ||||||
| found = False | ||||||
|
Comment on lines
+132
to
133
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Protect fallback from invalid folder formats. If Proposed fix- alt_folders = legacy_folder_alternates(folder)
+ try:
+ alt_folders = legacy_folder_alternates(folder)
+ except ValueError:
+ alt_folders = []🤖 Prompt for AI Agents |
||||||
| for alt in alt_folders: | ||||||
| alt_source = (media_root / alt / filename).resolve() | ||||||
| if alt_source.is_relative_to(media_root_resolved) and alt_source.exists(): | ||||||
| source = alt_source | ||||||
| found = True | ||||||
| break | ||||||
| try: | ||||||
| alt_source = (media_root / alt / filename).resolve() | ||||||
Check failureCode scanning / CodeQL Uncontrolled data used in path expression High
This path depends on a
user-provided value Error loading related location Loading This path depends on a user-provided value Error loading related location Loading |
||||||
|
|
||||||
| if alt_source.is_relative_to(media_root_resolved) and alt_source.exists(): | ||||||
Check failureCode scanning / CodeQL Uncontrolled data used in path expression High
This path depends on a
user-provided value Error loading related location Loading This path depends on a user-provided value Error loading related location Loading |
||||||
|
|
||||||
| logger.debug("Thumbnail legacy fallback resolved via alternate folder") | ||||||
| source = alt_source | ||||||
| resolved_folder = alt | ||||||
| found = True | ||||||
| break | ||||||
| except OSError, RuntimeError: | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
rg -nP 'except\s+[^(\n:]+,\s*[^:\n]+:' src/web/main.py src/web/thumbnails.pyRepository: GeiserX/Telegram-Archive Length of output: 876 🏁 Script executed: #!/bin/bash
set -euo pipefail
# Show the exact code around the reported line
nl -ba src/web/thumbnails.py | sed -n '120,170p'
# Find all occurrences of the specific invalid syntax pattern in the repo
rg -n "except OSError, RuntimeError" -S src/web
# Check declared Python version / requirements (if present)
ls -la
rg -n "python_requires|Programming Language :: Python|requires-python" -S pyproject.toml setup.cfg setup.py requirements*.txt .github/workflows 2>/dev/null || trueRepository: GeiserX/Telegram-Archive Length of output: 111 🏁 Script executed: #!/bin/bash
set -euo pipefail
# Code context around the reported line
cat -n src/web/thumbnails.py | sed -n '120,165p'
# Locate the exact offending syntax in the repo
rg -n "except OSError, RuntimeError" -S src/web || true
# Show declared Python version (if any)
rg -n "python_requires|Programming Language :: Python|requires-python" -S pyproject.toml setup.cfg setup.py requirements*.txt .github/workflows 2>/dev/null || true
# Parse the file as Python 3 to check syntax validity
python3 - <<'PY'
import ast, pathlib, sys
p = pathlib.Path("src/web/thumbnails.py")
src = p.read_text(encoding="utf-8")
try:
ast.parse(src)
print("PY3_AST_PARSE_OK")
except SyntaxError as e:
print("PY3_AST_PARSE_SYNTAX_ERROR")
print(e)
PYRepository: GeiserX/Telegram-Archive Length of output: 1871 Fix invalid multi-exception syntax in Suggested fix- except OSError, RuntimeError:
+ except (OSError, RuntimeError):📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||
| continue | ||||||
| if not found: | ||||||
| return None | ||||||
|
|
||||||
| async with _generation_semaphore: | ||||||
| loop = asyncio.get_running_loop() | ||||||
| ok = await loop.run_in_executor(None, _generate_sync, source, dest, size) | ||||||
| return dest if ok else None | ||||||
| return (dest, resolved_folder) if ok else None | ||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
Repository: GeiserX/Telegram-Archive
Length of output: 876
🏁 Script executed:
Repository: GeiserX/Telegram-Archive
Length of output: 111
🏁 Script executed:
Repository: GeiserX/Telegram-Archive
Length of output: 1636
🏁 Script executed:
Repository: GeiserX/Telegram-Archive
Length of output: 7691
Fix Python 3–invalid
exceptsyntax (line 938).src/web/main.pyuses Python-2 comma exception syntax:except OSError, ValueError, RuntimeError:which won’t parse under the project’srequires-python >= 3.14. Use tuple form; this comma-except pattern also appears elsewhere (e.g.,src/web/main.py:925,src/web/thumbnails.py:141).Diff
📝 Committable suggestion
🤖 Prompt for AI Agents