Update main.py #11
Update main.py #11
Conversation
📝 WalkthroughWalkthroughThis change replaces a minimal TeleBot setup with a full-featured python-telegram-bot v20 implementation. It introduces asynchronous handlers, persistent JSON data stores, admin/user workflows, channel membership verification, file management with upload/broadcast capabilities, and comprehensive keyboard navigation for managing free/paid files and user-admin interactions. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Bot
participant Channel
participant DataStore
participant Admin
User->>Bot: /start
Bot->>Channel: Check membership
alt Member
Channel-->>Bot: Member verified
Bot->>DataStore: Load user list
DataStore-->>Bot: User data
Bot->>Bot: Add/track user
Bot-->>User: Show main menu
else Not Member
Channel-->>Bot: Not member
Bot-->>User: Show join keyboard
end
sequenceDiagram
actor Admin
participant Bot
participant DataStore
participant Users
Admin->>Bot: Upload file (free/paid)
Bot->>DataStore: Store file metadata
DataStore-->>Bot: Stored
Bot->>Bot: Broadcast to users
loop For each user
Bot->>Users: Send file notification
Users-->>Bot: Ack
end
Bot-->>Admin: Broadcast complete (count)
sequenceDiagram
actor User
participant Bot
participant FileStore
participant Admin
User->>Bot: Browse files
Bot->>FileStore: Fetch file list
FileStore-->>Bot: File metadata
Bot-->>User: Display files
User->>Bot: Request paid file
Bot->>Admin: Forward request
Admin-->>User: Admin confirmation
alt Approved
User->>Bot: Download/receive
else Denied
Admin-->>User: Rejection message
end
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~70 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (4)
main.py (4)
98-99: Unused parameteris_admin_user.The parameter is defined but never used in the function body. Either remove it or implement the intended differentiated behavior.
🔎 Proposed fix
-def back_button(is_admin_user=False): +def back_button(): return InlineKeyboardMarkup([[InlineKeyboardButton("🔙 Back", callback_data="back_main")]])
130-135: Unused parametersdescription,price,fidin broadcast notification.These parameters are passed but never used. The broadcast message doesn't include file description or price for paid files, which limits the notification's usefulness.
🔎 Proposed fix to include useful information
async def broadcast_new_file(context, file_name, file_type, description=None, price=None, fid=None): users = load_users() - text = f"📢 New {file_type} file: {file_name}" + if file_type == "paid" and price: + text = f"📢 New {file_type} file: {file_name}\n💰 Price: ${price}" + if description: + text += f"\n📝 {description}" + else: + text = f"📢 New {file_type} file: {file_name}" for uid in users: - try: await context.bot.send_message(uid, text) - except: continue + try: + await context.bot.send_message(uid, text) + except Exception: + continue
293-298: Missingreturnafter handling message to admin.Other branches in this function use
returnafter completing their action, but this path doesn't. While not currently causing issues, addingreturnwould be consistent and prevent accidental fall-through if code is added later.🔎 Proposed fix
if context.user_data.get("msg_to_admin"): if uid in load(CLOSED_CHATS_DB): await update.message.reply_text("❌ Chat is closed.") return await context.bot.send_message(OWNER_ID, f"📩 Message from user {uid}:\n{text}", reply_markup=close_chat_kb()) await update.message.reply_text("✅ Sent to admin.") + return
10-15: Consider data integrity for concurrent operations.JSON file storage with read-modify-write patterns can lead to data loss under concurrent access. While
run_pollingprocesses updates sequentially, operations like broadcasting (broadcast_new_file) run concurrently with the main handler loop. For production use, consider using SQLite with proper transactions or file locking.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
main.py
🧰 Additional context used
🪛 Ruff (0.14.10)
main.py
45-45: Consider moving this statement to an else block
(TRY300)
46-46: Do not use bare except
(E722)
98-98: Unused function argument: is_admin_user
(ARG001)
120-120: Multiple statements on one line (colon)
(E701)
125-125: Multiple statements on one line (semicolon)
(E702)
127-127: Multiple statements on one line (colon)
(E701)
128-128: Do not use bare except
(E722)
128-128: Multiple statements on one line (colon)
(E701)
130-130: Unused function argument: description
(ARG001)
130-130: Unused function argument: price
(ARG001)
130-130: Unused function argument: fid
(ARG001)
134-134: Multiple statements on one line (colon)
(E701)
135-135: Do not use bare except
(E722)
135-135: try-except-continue detected, consider logging the exception
(S112)
135-135: Multiple statements on one line (colon)
(E701)
140-140: Multiple statements on one line (colon)
(E701)
141-141: Do not use bare except
(E722)
141-141: try-except-pass detected, consider logging the exception
(S110)
141-141: Multiple statements on one line (colon)
(E701)
148-148: Multiple statements on one line (colon)
(E701)
163-163: Multiple statements on one line (semicolon)
(E702)
176-176: f-string without any placeholders
Remove extraneous f prefix
(F541)
213-213: Multiple statements on one line (semicolon)
(E702)
216-216: Do not use bare except
(E722)
216-216: Multiple statements on one line (colon)
(E701)
220-220: Multiple statements on one line (colon)
(E701)
224-224: Multiple statements on one line (colon)
(E701)
224-224: Multiple statements on one line (semicolon)
(E702)
226-226: Do not use bare except
(E722)
226-226: try-except-pass detected, consider logging the exception
(S110)
226-226: Multiple statements on one line (colon)
(E701)
229-229: Multiple statements on one line (colon)
(E701)
233-233: Multiple statements on one line (colon)
(E701)
233-233: Multiple statements on one line (semicolon)
(E702)
235-235: Do not use bare except
(E722)
235-235: try-except-pass detected, consider logging the exception
(S110)
235-235: Multiple statements on one line (colon)
(E701)
238-238: Multiple statements on one line (colon)
(E701)
240-240: Multiple statements on one line (colon)
(E701)
246-246: Multiple statements on one line (semicolon)
(E702)
246-246: Multiple statements on one line (semicolon)
(E702)
247-247: Multiple statements on one line (semicolon)
(E702)
258-258: Multiple statements on one line (semicolon)
(E702)
258-258: Multiple statements on one line (semicolon)
(E702)
259-259: Multiple statements on one line (semicolon)
(E702)
261-261: Do not use bare except
(E722)
261-261: Multiple statements on one line (colon)
(E701)
277-277: Multiple statements on one line (colon)
(E701)
277-277: Multiple statements on one line (semicolon)
(E702)
278-278: Do not use bare except
(E722)
278-278: try-except-continue detected, consider logging the exception
(S112)
278-278: Multiple statements on one line (colon)
(E701)
290-290: Consider moving this statement to an else block
(TRY300)
291-291: Do not use bare except
(E722)
291-291: try-except-pass detected, consider logging the exception
(S110)
291-291: Multiple statements on one line (colon)
(E701)
🔇 Additional comments (1)
main.py (1)
300-310: Handler registration looks correct, but verify paid file flow.The handler ordering is appropriate: documents are handled by
handle_upload, and text messages bymessage_router. However, as noted above, the paid file details flow is broken becausehandle_paid_detailsis never invoked. Ensure the fix integrates that logic intomessage_router.
| for db in [FILES_DB, ADMINS_DB, USERS_DB, MESSAGES_DB, CLOSED_CHATS_DB]: | ||
| if not os.path.exists(db): | ||
| if db == ADMINS_DB: | ||
| json.dump([OWNER_ID], open(db, "w", encoding="utf-8")) | ||
| elif db == CLOSED_CHATS_DB: | ||
| json.dump([], open(db, "w", encoding="utf-8")) | ||
| else: | ||
| json.dump([], open(db, "w", encoding="utf-8")) |
There was a problem hiding this comment.
Resource leak: file handles not closed; redundant conditional branch.
The open() calls return file handles that are never explicitly closed. Use context managers or the existing save() function. Also, lines 25-28 are redundant since both branches write [].
🔎 Proposed fix
for db in [FILES_DB, ADMINS_DB, USERS_DB, MESSAGES_DB, CLOSED_CHATS_DB]:
if not os.path.exists(db):
if db == ADMINS_DB:
- json.dump([OWNER_ID], open(db, "w", encoding="utf-8"))
- elif db == CLOSED_CHATS_DB:
- json.dump([], open(db, "w", encoding="utf-8"))
+ save(db, [OWNER_ID])
else:
- json.dump([], open(db, "w", encoding="utf-8"))
+ save(db, [])Note: This requires moving the save() function definition before this block, or using an inline context manager:
with open(db, "w", encoding="utf-8") as f:
json.dump([OWNER_ID] if db == ADMINS_DB else [], f)Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In main.py around lines 21 to 28 the code opens files with json.dump(open(...))
creating unclosed file handles and contains redundant branches that both write
empty lists; fix by using a with-context manager for each db file and write
either [OWNER_ID] when db == ADMINS_DB or [] otherwise (or call the
already-defined save() helper if you move its definition above this block),
ensuring files are properly closed after writing.
| async def is_member(bot, uid): | ||
| try: | ||
| m = await bot.get_chat_member(FORCE_CHANNEL, uid) | ||
| return m.status in ["member", "administrator", "creator"] | ||
| except: | ||
| return False |
There was a problem hiding this comment.
Avoid bare except:; catch specific exceptions.
Bare except: catches all exceptions including SystemExit and KeyboardInterrupt. This masks real errors and makes debugging difficult. Catch specific Telegram API exceptions.
🔎 Proposed fix
+from telegram.error import TelegramError
+
async def is_member(bot, uid):
try:
m = await bot.get_chat_member(FORCE_CHANNEL, uid)
return m.status in ["member", "administrator", "creator"]
- except:
+ except TelegramError:
return FalseCommittable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 Ruff (0.14.10)
45-45: Consider moving this statement to an else block
(TRY300)
46-46: Do not use bare except
(E722)
🤖 Prompt for AI Agents
In main.py around lines 42 to 47, replace the bare "except:" with catching
specific exceptions from the Telegram/AIO libraries; for example catch
telegram.error.TelegramError (or the equivalent exception class used by your bot
library) and asyncio.TimeoutError and return False for those cases, and for any
other unexpected exception either log it and re-raise or let it propagate so it
isn't silently swallowed.
| elif q.data.startswith("get_"): | ||
| fid = q.data.replace("get_", "") | ||
| for f in data: | ||
| if f["id"] == fid: | ||
| path = f"files/free/{f['name']}" | ||
| f["downloads"] += 1; save(FILES_DB, data) | ||
| await q.message.reply_document(open(path, "rb")) | ||
| return |
There was a problem hiding this comment.
Path traversal vulnerability and resource leak.
- Path traversal: The filename from JSON is used directly without sanitization. A malicious filename like
../../etc/passwdcould access files outside the intended directory. - Resource leak:
open(path, "rb")creates a file handle that's never closed. - Incorrect order: Download count is incremented before confirming the file was sent successfully.
🔎 Proposed fix
elif q.data.startswith("get_"):
fid = q.data.replace("get_", "")
for f in data:
if f["id"] == fid:
- path = f"files/free/{f['name']}"
- f["downloads"] += 1; save(FILES_DB, data)
- await q.message.reply_document(open(path, "rb"))
- return
+ # Sanitize filename to prevent path traversal
+ safe_name = os.path.basename(f['name'])
+ path = f"files/free/{safe_name}"
+ if not os.path.isfile(path):
+ await q.answer("File not found", show_alert=True)
+ return
+ with open(path, "rb") as file:
+ await q.message.reply_document(file)
+ f["downloads"] += 1
+ save(FILES_DB, data)
+ returnCommittable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 Ruff (0.14.10)
163-163: Multiple statements on one line (semicolon)
(E702)
| for f in data: | ||
| if f["id"] == fid: | ||
| await context.bot.send_message(OWNER_ID, f"📥 Paid file request from user {uid}:\nFile: {f['name']}\nPrice: ${f['price']}") | ||
| await q.message.reply_text(f"✅ Purchase request sent to admin.") |
There was a problem hiding this comment.
f-string without placeholders.
This f-string has no placeholders, making the f prefix unnecessary.
🔎 Proposed fix
- await q.message.reply_text(f"✅ Purchase request sent to admin.")
+ await q.message.reply_text("✅ Purchase request sent to admin.")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| await q.message.reply_text(f"✅ Purchase request sent to admin.") | |
| await q.message.reply_text("✅ Purchase request sent to admin.") |
🧰 Tools
🪛 Ruff (0.14.10)
176-176: f-string without any placeholders
Remove extraneous f prefix
(F541)
🤖 Prompt for AI Agents
In main.py around line 176, the string uses an f-string with no placeholders
(f"✅ Purchase request sent to admin.") — remove the unnecessary f prefix and
replace it with a normal string literal ("✅ Purchase request sent to admin.") to
avoid misleading usage of f-strings; simply edit that line to drop the leading f
and keep the existing text and quoting.
| elif q.data == "close_this_chat" and uid == OWNER_ID: | ||
| try: | ||
| target_uid = int(q.message.text.split("user ")[1].split(":")[0].strip()) | ||
| closed = load(CLOSED_CHATS_DB) | ||
| if target_uid not in closed: | ||
| closed.append(target_uid); save(CLOSED_CHATS_DB, closed) | ||
| await q.message.edit_text(q.message.text + "\n\n🔒 (CHAT CLOSED)") | ||
| await context.bot.send_message(target_uid, "🚫 Admin closed this chat.") | ||
| except: await q.answer("Error") |
There was a problem hiding this comment.
Fragile parsing of user ID from message text.
Extracting the user ID by string parsing (split("user ")[1].split(":")[0]) is error-prone. If the message format changes or contains unexpected text, this will fail silently. Consider storing user context in context.user_data or message metadata instead.
🧰 Tools
🪛 Ruff (0.14.10)
213-213: Multiple statements on one line (semicolon)
(E702)
216-216: Do not use bare except
(E722)
216-216: Multiple statements on one line (colon)
(E701)
🤖 Prompt for AI Agents
In main.py around lines 208 to 216, the code brittlely extracts the target user
id with q.message.text.split("user ")[1].split(":")[0], which will break if the
message format changes; replace this fragile parsing by retrieving the user id
from reliable sources (preferably: a) the replied-to message's from_user.id if
the command is used as a reply, or b) a stored value in context.user_data when
creating the message, or c) parse with a strict regex and int() validation),
validate that the parsed value is an int before using it, and wrap only the
minimal risky operations in try/except that logs the actual exception and
returns a clear error to the admin instead of a silent generic error.
| async def add_admin(update, context): | ||
| if update.effective_user.id != OWNER_ID: return | ||
| try: | ||
| uid = int(context.args[0]) | ||
| admins = load(ADMINS_DB) | ||
| if uid not in admins: admins.append(uid); save(ADMINS_DB, admins) | ||
| await update.message.reply_text("✅ Admin added") | ||
| except: pass | ||
|
|
||
| async def del_admin(update, context): | ||
| if update.effective_user.id != OWNER_ID: return | ||
| try: | ||
| uid = int(context.args[0]) | ||
| admins = load(ADMINS_DB) | ||
| if uid in admins and uid != OWNER_ID: admins.remove(uid); save(ADMINS_DB, admins) | ||
| await update.message.reply_text("✅ Admin removed") | ||
| except: pass |
There was a problem hiding this comment.
Bare except: pass silently swallows all errors including invalid input.
Using except: pass hides errors completely. If an admin provides invalid input, they receive no feedback. At minimum, catch specific exceptions and provide user feedback.
🔎 Proposed fix for add_admin
async def add_admin(update, context):
- if update.effective_user.id != OWNER_ID: return
+ if update.effective_user.id != OWNER_ID:
+ return
try:
uid = int(context.args[0])
admins = load(ADMINS_DB)
- if uid not in admins: admins.append(uid); save(ADMINS_DB, admins)
+ if uid not in admins:
+ admins.append(uid)
+ save(ADMINS_DB, admins)
await update.message.reply_text("✅ Admin added")
- except: pass
+ except (IndexError, ValueError):
+ await update.message.reply_text("❌ Usage: /addadmin USER_ID")🧰 Tools
🪛 Ruff (0.14.10)
220-220: Multiple statements on one line (colon)
(E701)
224-224: Multiple statements on one line (colon)
(E701)
224-224: Multiple statements on one line (semicolon)
(E702)
226-226: Do not use bare except
(E722)
226-226: try-except-pass detected, consider logging the exception
(S110)
226-226: Multiple statements on one line (colon)
(E701)
229-229: Multiple statements on one line (colon)
(E701)
233-233: Multiple statements on one line (colon)
(E701)
233-233: Multiple statements on one line (semicolon)
(E702)
235-235: Do not use bare except
(E722)
235-235: try-except-pass detected, consider logging the exception
(S110)
235-235: Multiple statements on one line (colon)
(E701)
🤖 Prompt for AI Agents
In main.py around lines 219 to 235, the handlers use bare "except: pass" which
silently swallows errors and gives no feedback; replace the broad excepts with
specific exception handling: catch IndexError and ValueError to detect missing
or non-integer arguments and reply to the user with an error message, and
optionally catch (and log) IOError/Exception around load/save to report
persistence problems; ensure you only save when the admins list actually changes
and still enforce OWNER_ID check at the top.
| async def handle_upload(update, context): | ||
| if not is_admin(update.effective_user.id): return | ||
| upload_type = context.user_data.get("upload") | ||
| if not upload_type or not update.message.document: return | ||
| doc = update.message.document | ||
| fid = uuid.uuid4().hex[:6] | ||
| path = f"files/{upload_type}/{doc.file_name}" | ||
| await (await doc.get_file()).download_to_drive(path) | ||
| if upload_type == "free": | ||
| data = load_files(); data.append({"id":fid,"name":doc.file_name,"type":"free","downloads":0}); save(FILES_DB, data) | ||
| await update.message.reply_text("✅ Uploaded"); await broadcast_new_file(context, doc.file_name, "free") | ||
| context.user_data.clear() | ||
| else: | ||
| context.user_data["paid_file"] = {"id":fid,"file_path":path,"type":"paid"} | ||
| await update.message.reply_text("💰 Name | Description | Price") |
There was a problem hiding this comment.
Path traversal vulnerability in file upload.
The doc.file_name is used directly in the path without sanitization. A malicious file named ../../../important_file.txt could overwrite files outside the intended directory.
🔎 Proposed fix
async def handle_upload(update, context):
if not is_admin(update.effective_user.id): return
upload_type = context.user_data.get("upload")
if not upload_type or not update.message.document: return
doc = update.message.document
fid = uuid.uuid4().hex[:6]
- path = f"files/{upload_type}/{doc.file_name}"
+ # Sanitize filename to prevent path traversal
+ safe_name = os.path.basename(doc.file_name)
+ path = f"files/{upload_type}/{safe_name}"
await (await doc.get_file()).download_to_drive(path)
if upload_type == "free":
- data = load_files(); data.append({"id":fid,"name":doc.file_name,"type":"free","downloads":0}); save(FILES_DB, data)
+ data = load_files()
+ data.append({"id": fid, "name": safe_name, "type": "free", "downloads": 0})
+ save(FILES_DB, data)Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 Ruff (0.14.10)
238-238: Multiple statements on one line (colon)
(E701)
240-240: Multiple statements on one line (colon)
(E701)
246-246: Multiple statements on one line (semicolon)
(E702)
246-246: Multiple statements on one line (semicolon)
(E702)
247-247: Multiple statements on one line (semicolon)
(E702)
🤖 Prompt for AI Agents
In main.py around lines 237 to 251, the code uses doc.file_name directly to
build the filesystem path which allows path traversal (e.g., "../../../file")
and can overwrite arbitrary files; sanitize and validate the uploaded filename
before saving: strip any directory components (use os.path.basename or
equivalent), validate/whitelist allowed characters or extensions, prefix or
replace the name with the generated fid (e.g., fid + "_" + safe_name) to avoid
collisions, build the path with os.path.join against a fixed base directory and
verify the resulting normalized absolute path starts with that base directory to
prevent traversal, ensure the target directory exists (os.makedirs(...,
exist_ok=True)), and reject or rename inputs that do not pass validation.
| async def handle_paid_details(update, context): | ||
| try: | ||
| name, desc, price = [x.strip() for x in update.message.text.split("|")] | ||
| finfo = context.user_data["paid_file"] | ||
| finfo.update({"name":name,"description":desc,"price":price,"downloads":0}) | ||
| data = load_files(); data.append(finfo); save(FILES_DB, data) | ||
| await update.message.reply_text("✅ Paid File Saved"); await broadcast_new_file(context, name, "paid", desc, price, finfo["id"]) | ||
| context.user_data.clear() | ||
| except: await update.message.reply_text("❌ Format: Name | Description | Price") |
There was a problem hiding this comment.
handle_paid_details is never registered as a handler.
This function is defined but not added to the application handlers. When an admin sends the "Name | Description | Price" text, it will be caught by message_router instead, which doesn't call this function. The paid file upload flow is broken.
🔎 Proposed fix - integrate into message_router or register separately
Add logic to message_router to detect and handle paid file details:
async def message_router(update: Update, context: ContextTypes.DEFAULT_TYPE):
uid = update.effective_user.id
text = update.message.text or ""
# واکنش به دکمه بازگشت کنار سنجاق
if text == "🔙 Back to Main Menu":
context.user_data.clear()
await update.message.reply_text("🔹 Main Menu", reply_markup=user_menu(is_admin(uid)))
return
+ # Handle paid file details input
+ if context.user_data.get("paid_file") and is_admin(uid):
+ await handle_paid_details(update, context)
+ return
+
if context.user_data.get("broadcasting") and is_admin(uid):Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 Ruff (0.14.10)
258-258: Multiple statements on one line (semicolon)
(E702)
258-258: Multiple statements on one line (semicolon)
(E702)
259-259: Multiple statements on one line (semicolon)
(E702)
261-261: Do not use bare except
(E722)
261-261: Multiple statements on one line (colon)
(E701)
🤖 Prompt for AI Agents
In main.py around lines 253 to 261, the async handler handle_paid_details is
defined but never registered so admin "Name | Description | Price" messages are
routed to message_router instead; fix by either (A) registering
handle_paid_details with the bot/application message handlers (only for admins
or when context.user_data contains "paid_file") so it receives messages matching
the pipe-separated pattern, or (B) updating message_router to detect the
pipe-separated "Name | Description | Price" format and the presence of
context.user_data["paid_file"] and call await handle_paid_details(update,
context); ensure the handler is only invoked for authorized admin users, and
that any necessary imports/registrations are added where other handlers are
registered.
1 participant
Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.