Skip to content

feat(openapi): add GET file endpoint and update file route to support multiple methods #6874

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.

Sign up for GitHub

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

Merged
Soulter merged 1 commit into from
Mar 24, 2026
Merged

feat(openapi): add GET file endpoint and update file route to support multiple methods #6874

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
13 changes: 8 additions & 5 deletions astrbot/dashboard/routes/open_api.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,10 @@ def __init__(
"/v1/chat": ("POST", self.chat_send),
"/v1/chat/sessions": ("GET", self.get_chat_sessions),
"/v1/configs": ("GET", self.get_chat_configs),
"/v1/file": ("POST", self.upload_file),
"/v1/file": [
("POST", self.upload_file),
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot Mar 24, 2026

Choose a reason for hiding this comment

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

security-critical critical

The self.chat_route.post_file() method called by upload_file has a path traversal vulnerability. It uses the user-provided filename directly when saving the uploaded file. An attacker could provide a malicious filename like ../../../../etc/passwd to write files outside of the intended directory.

This is a critical security issue.

To fix this, you should sanitize the filename using werkzeug.utils.secure_filename before saving it. The fix should be applied in the post_file method in astrbot/dashboard/routes/chat.py.

Example fix in chat.py:

from werkzeug.utils import secure_filename
...
async def post_file(self):
    ...
    file = post_data["file"]
    if file.filename:
        filename = secure_filename(file.filename)
    else:
        filename = f"{uuid.uuid4()!s}"
    ...
    path = os.path.join(self.attachments_dir, filename)
    await file.save(path)
    ...

("GET", self.get_file),
],
"/v1/im/message": ("POST", self.send_message),
"/v1/im/bots": ("GET", self.get_bots),
}
Expand Down Expand Up @@ -455,10 +458,7 @@ async def _handle_chat_ws_send(self, post_data: dict) -> None:
if msg_type == "end":
break
if (streaming and msg_type == "complete") or not streaming:
if chain_type in (
"tool_call",
"tool_call_result",
):
if chain_type in ("tool_call", "tool_call_result"):
continue
try:
refs = self.chat_route._extract_web_search_refs(
Expand Down Expand Up @@ -540,6 +540,9 @@ async def chat_ws(self) -> None:
async def upload_file(self):
return await self.chat_route.post_file()

async def get_file(self):
return await self.chat_route.get_attachment()
Comment on lines +543 to +544
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot Mar 24, 2026

Choose a reason for hiding this comment

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

security-critical critical

The method self.chat_route.get_attachment() that is being called here has a path traversal vulnerability. It retrieves a file path from the database and serves it without validating that the path is within the expected attachments directory. An attacker with the ability to manipulate attachment data in the database could read arbitrary files on the server.

This is a critical security issue.

To fix this, you should validate the path before serving the file. It's best to fix this in get_attachment in astrbot/dashboard/routes/chat.py. Since that file is not in this PR, you can apply a fix here for the new endpoint. This will require adding import os and modifying the quart import to include send_file.

Here is a suggestion for a secure implementation of this method:

async def get_file(self):
    attachment_id = request.args.get("attachment_id")
    if not attachment_id:
        return Response().error("Missing key: attachment_id").__dict__

    try:
        attachment = await self.db.get_attachment_by_id(attachment_id)
        if not attachment:
            return Response().error("Attachment not found").__dict__

        file_path = attachment.path
        real_file_path = os.path.realpath(file_path)

        # Security check for path traversal
        attachments_dir = os.path.realpath(self.chat_route.attachments_dir)
        legacy_img_dir = os.path.realpath(self.chat_route.legacy_img_dir)
        
        is_safe = real_file_path.startswith(attachments_dir) or real_file_path.startswith(legacy_img_dir)

        if not is_safe:
            return Response().error("Invalid file path").__dict__

        return await send_file(real_file_path, mimetype=attachment.mime_type)

    except (FileNotFoundError, OSError):
        return Response().error("File access error").__dict__

Please also consider fixing the get_attachment method in astrbot/dashboard/routes/chat.py and other similar file serving methods.


async def get_chat_sessions(self):
username, username_err = self._resolve_open_username(
request.args.get("username")
Expand Down
42 changes: 42 additions & 0 deletions docs/public/openapi.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,48 @@
"403": {
"$ref": "#/components/responses/Forbidden"
}
}
},
"get": {
"tags": [
"Open API"
],
"summary": "Get attachment file",
"description": "Get an uploaded attachment file by attachment_id.",
"security": [
{
"ApiKeyHeader": []
}
],
"parameters": [
{
"name": "attachment_id",
"in": "query",
"required": true,
"schema": {
"type": "string"
},
"description": "Attachment ID returned by POST /api/v1/file."
}
],
"responses": {
"200": {
"description": "OK",
"content": {
"application/octet-stream": {
"schema": {
"type": "string",
"format": "binary"
}
}
}
},
"401": {
"$ref": "#/components/responses/Unauthorized"
},
"403": {
"$ref": "#/components/responses/Forbidden"
}
}
}
},
Expand Down
Loading
Loading