Skip to content

fix(computer): send sandbox image downloads as images #7785

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
Apr 25, 2026
+17 −3
Merged

fix(computer): send sandbox image downloads as images #7785

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
20 changes: 17 additions & 3 deletions astrbot/core/tools/computer_tools/fs.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@
from astrbot.core.astr_agent_context import AstrAgentContext
from astrbot.core.computer.computer_client import get_booter
from astrbot.core.computer.file_read_utils import read_file_tool_result
from astrbot.core.message.components import File
from astrbot.core.message.components import File, Image
from astrbot.core.utils.astrbot_path import (
get_astrbot_skills_path,
get_astrbot_system_tmp_path,
Expand All @@ -64,6 +64,7 @@
_SANDBOX_RUNTIME_TOOL_CONFIG = {
"provider_settings.computer_use_runtime": "sandbox",
}
_IMAGE_FILE_SUFFIXES = {".bmp", ".gif", ".jpeg", ".jpg", ".png", ".webp"}


def _restricted_env_path_labels(umo: str) -> list[str]:
Expand Down Expand Up @@ -729,19 +730,32 @@ async def call(
if also_send_to_user:
try:
name = os.path.basename(local_path)
if Path(local_path).suffix.lower() in _IMAGE_FILE_SUFFIXES:
message_component = Image.fromFileSystem(local_path)
sent_as = "image"
else:
message_component = File(name=name, file=local_path)
sent_as = "file"
Comment on lines +733 to +738
Copy link
Copy Markdown
Contributor

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

Choose a reason for hiding this comment

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

medium

The logic for determining the message component based on the file extension is correct. However, since os.path is already heavily used in this function (e.g., os.path.basename on line 732), you might consider using os.path.splitext(local_path)[1].lower() to avoid creating a Path object just for the suffix check, which would be slightly more efficient and consistent with the surrounding code. Additionally, if this logic for handling attachments is implemented similarly for different cases (e.g., direct vs. quoted attachments), please refactor it into a shared helper function to avoid code duplication.

                    suffix = os.path.splitext(local_path)[1].lower()
                    if suffix in _IMAGE_FILE_SUFFIXES:
                        message_component = Image.fromFileSystem(local_path)
                        sent_as = "image"
                    else:
                        message_component = File(name=name, file=local_path)
                        sent_as = "file"
References
  1. When implementing similar functionality for different cases (e.g., direct vs. quoted attachments), refactor the logic into a shared helper function to avoid code duplication.

await context.context.event.send(
MessageChain(chain=[File(name=name, file=local_path)])
MessageChain(chain=[message_component])
)
except Exception as e:
logger.error(f"Error sending file message: {e}")
return (
f"File downloaded successfully to {local_path} "
Comment on lines 743 to +745
Copy link
Copy Markdown
Contributor

@sourcery-ai sourcery-ai Bot Apr 24, 2026

Choose a reason for hiding this comment

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

🚨 issue (security): Returning the full local path and raw exception message may expose internal details to the user.

Consider returning a user-friendly error and logging the full path/exception instead, or redacting sensitive parts of the path and exception before including them in the user-visible message.

f"but sending to user failed: {e}"
)

# remove
# try:
# os.remove(local_path)
# except Exception as e:
# logger.error(f"Error removing temp file {local_path}: {e}")

return f"File downloaded successfully to {local_path} and sent to user."
return (
f"File downloaded successfully to {local_path} "
f"and sent to user as {sent_as}."
)

return f"File downloaded successfully to {local_path}"
except Exception as e:
Expand Down
Loading