Skip to content
Open
Changes from 3 commits
Commits
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
15 changes: 10 additions & 5 deletions astrbot/core/platform/sources/webchat/webchat_event.py
Original file line number Diff line number Diff line change
Expand Up @@ -109,14 +109,19 @@ async def _send(
},
)
elif isinstance(comp, File):
# save file to local
# save file to local with original name
file_path = await comp.get_file()
original_name = comp.name or os.path.basename(file_path)
Comment thread
sourcery-ai[bot] marked this conversation as resolved.
ext = os.path.splitext(original_name)[1] or ""
filename = f"{uuid.uuid4()!s}{ext}"
dest_path = os.path.join(attachments_dir, filename)
safe_name = os.path.basename(original_name)
dest_path = os.path.join(attachments_dir, safe_name)
name_part, ext_part = os.path.splitext(safe_name)
# dedup: if file with same name exists, append a counter
counter = 1
while os.path.exists(dest_path):
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

这里可以限制循环上限(比如 100_000),防止一些奇怪的问题

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

谢谢大佬!空了麻烦您再帮忙看看。

起初我只是觉得AI每次回复的文件都被重命名,就很不方便,
于是我问AstrBot,帮忙修改实现的这个效果,

因为我完全没有任何编程经验,我判断不了这个修改是否符合项目的规范,可能会带来哪些风险。

dest_path = os.path.join(attachments_dir, f"{name_part}_{counter}{ext_part}")
counter += 1
Comment on lines 114 to +125
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

security-high high

Security & Efficiency Review

  1. Path Traversal Vulnerability (High Severity): Using comp.name directly in os.path.join allows potential path traversal attacks if the filename contains directory traversal sequences (e.g., ../ or ..\). This could allow an attacker to write files outside the intended attachments_dir directory. Wrapping it in os.path.basename sanitizes the filename.
  2. Redundant Computation: Calling os.path.splitext(original_name) inside the while loop is redundant because original_name does not change. Moving it outside the loop improves performance.
Suggested change
original_name = comp.name or os.path.basename(file_path)
ext = os.path.splitext(original_name)[1] or ""
filename = f"{uuid.uuid4()!s}{ext}"
dest_path = os.path.join(attachments_dir, filename)
dest_path = os.path.join(attachments_dir, original_name)
# dedup: if file with same name exists, append a counter
counter = 1
while os.path.exists(dest_path):
name_part, ext_part = os.path.splitext(original_name)
dest_path = os.path.join(attachments_dir, f"{name_part}_{counter}{ext_part}")
counter += 1
original_name = os.path.basename(comp.name or file_path)
name_part, ext_part = os.path.splitext(original_name)
dest_path = os.path.join(attachments_dir, original_name)
# dedup: if file with same name exists, append a counter
counter = 1
while os.path.exists(dest_path):
dest_path = os.path.join(attachments_dir, f"{name_part}_{counter}{ext_part}")
counter += 1

shutil.copy2(file_path, dest_path)
data = f"[FILE]{filename}"
data = f"[FILE]{os.path.basename(dest_path)}"
await web_chat_back_queue.put(
{
"type": "file",
Expand Down
Loading